Jump to content

Recommended Posts

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'];
    }

}

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?

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.

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.

  • Like 1

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. 

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.

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.