adamlang22 Posted October 7, 2015 Share Posted October 7, 2015 I have been using the below method to access my database for some time now and just wanted to make sure im doing the correct thing. My application runs perfectly but im just trying to improve my programming skills. The classes are all loaded in with an autoloader so in theory using the classes below I could just call: echo User::getUsername(1); Like I say it works fine I would just love some feedback about what other people do and suggestions on something that might run better or looks cleaner. class Db { private static $db_read; private static $db_write; public static method read(){ if( self::$db_read == null ){ //create new database connection is it doesnt exist self::$db_read = new PDO(); } return self::$db_read; } public static method write() { if( self::$db_write == null ){ //create new database connection is it doesnt exist self::$db_write = new PDO(); } return self::$db_write; } class User { public static function getUsername($user_id){ $d = Db::read()->prepare('select * from user where id = ? '); $d->execute( array($user_id) ); $user = $d->fetch(); return $user['username']; } } Quote Link to comment Share on other sites More sharing options...
ginerjm Posted October 7, 2015 Share Posted October 7, 2015 Without looking at your code, I am curious about your question. Whatever do you mean by asking what the "correct method of sharing a database object/connection"? A script opens a connection, uses it and it goes away when the script terminates. What is the "sharing" you are referring to? Quote Link to comment Share on other sites More sharing options...
adamlang22 Posted October 7, 2015 Author Share Posted October 7, 2015 Without looking at your code, I am curious about your question. Whatever do you mean by asking what the "correct method of sharing a database object/connection"? A script opens a connection, uses it and it goes away when the script terminates. What is the "sharing" you are referring to? Okay sorry I don't think I explained myself properly. I just wanted to make sure this was a good way that other classes could use the database rather than for example injecting the database object into a new class. And the fact I'm using static methods which I don't see too many people using and they instead instantiate the database class. Quote Link to comment Share on other sites More sharing options...
adamlang22 Posted October 7, 2015 Author Share Posted October 7, 2015 And for some reason on my Db class I wrote method instead of function????? Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted October 7, 2015 Share Posted October 7, 2015 This is the classical singleton approach. While it indeed works, it comes with huge problems: Your classes have ties to all kinds of global identifiers, which makes testing extremely difficult. You can't just take the individual User class and test it with a mock database object. You'd have to simulate the entire environment and point all Db, PDO etc. references to test classes. You're restricted to a single database connection once and forever. This can become a problem. For example, should you decide to store your PHP sessions in the database, you need two parallel connections. So you'd have to rewrite your whole database class and all classes depending on it. I'd take a different approach: Instead of passing a single database connection around, use a connection manager which supports an arbitrary number of named connections (e. g. “default” for the standard connection and “session” for the separate session connection). Instead of hard-coding the Db class, use Dependency Injection. You can either manually pass the connection manager to the User class, or you can use a Dependency Injection Container like Pimple. 1 Quote Link to comment Share on other sites More sharing options...
Strider64 Posted October 8, 2015 Share Posted October 8, 2015 I've since switched over to Dependency Injection, but here's a Singleton Class that I modified from a book by Larry Ullman ( A good place to learn PHP) class Database { private $_connection; // Store the single instance. private static $_instance; // Get an instance of the Database. // @return Database: public static function getInstance() { if (!self::$_instance) { self::$_instance = new self(); } return self::$_instance; } // Constructor - Build the PDO Connection: public function __construct() { $db_options = array( /* important! use actual prepared statements (default: emulate prepared statements) */ PDO::ATTR_EMULATE_PREPARES => false /* throw exceptions on errors (default: stay silent) */ , PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION /* fetch associative arrays (default: mixed arrays) */ , PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC ); $this->_connection = new PDO('mysql:host=' . DATABASE_HOST . ';dbname=' . DATABASE_NAME . ';charset=utf8', DATABASE_USERNAME, DATABASE_PASSWORD, $db_options); } // Empty clone magic method to prevent duplication: private function __clone() { } // Get the PDO connection: public function getConnection() { return $this->_connection; } } To use: $db = Database::getInstance(); $pdo = $db->getConnection(); I does get rid of some of the problems, but I already stated it's best to use D.I. for it's more versatile. Quote Link to comment Share on other sites More sharing options...
adamlang22 Posted October 8, 2015 Author Share Posted October 8, 2015 Sorry for the late reply, I really appreciate your input. I'm going to go and learn a lot more about dependency injection and the stuff about testing made sense. I need to start doing unit testing to move towards TDD which I think will present problems with my existing code and force me to improve my programming. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.