Singleton v Single Instance DB Connection in PHP

Question

I'm moving onto teaching myself OOP in PHP.

I'm creating a couple of little web apps and have followed a lot of tutorials that either create the database (using PDO) via a Singleton, or via passing the global around. I've read that these are pretty much the same thing and are both to be avoided like the plague.

So I've watched the Google Tech Talks on clean code, and read almost every SO article on dependency injection and the like. I have a couple of questions.

1. The clean code videos suggest you shouldn't do 'work' in your constructors. Is this 'work' in reference to business logic. Ie. If my class's job is to create another object, is that an OK kind of 'work'?

For example, in trying to conform to single repsonibility classes I created three.

1. Class DB - which actually connects to the database.
2. Class DBFactory - which creates the DB object which connects to the database.
3. Class DBInstance - which returns a single instance of the DBFactory created PDO object.

Please note that I'm trying to create a single instance, without creating a Singleton pattern.

So I try and pass my dependencies for each class up the chain. I find myself in a position where I have to create all of the objects (from DB down) so I can inject the dependencies. For some reason I thought it would work the other way, I'd create the first object, which would create the second for me etc. I'm clearly missing something?

Hopefully this helps others as well - there seems to be a myriad of questions relating to this stuff and databases but very little good examples.

(I should mention this does work, I do get a list of hotel names out of the database!)

TestCode.php

include './classes/DB.php';
include './classes/DBFactory.php';
include './classes/DBInstance.php';
include './classes/Location.php';

$db = new DB;
$dbfactory = new DBFactory($db);
$dbinstance = new DBInstance($dbfactory);
$dbh = $dbinstance->getDbInstance();

//Example business logic
$location_names = Location::getLocationNames($dbh);
print_r($location_names);

Class DB.php:

class DB {

    private $_dbhost = 'myhost';
    private $_dbname = 'myname';
    private $_dbuser = 'myuser';
    private $_dbpass = 'mypass';

    private $_error;

    public function connect() {
        try {
            return new PDO("mysql:host=$this->_dbhost;dbname=$this->_dbname", 
                $this->_dbuser, $this->_dbpass);
        }

        catch (PDOException $e) {
            $this->_error = 'Error! ' . $e->getMessage() . '<br />';
            die();
        }
    }

    public function getError() {
        if (isset($this->_error)) {
            return $this->_error;
        }
    }
}

Class DBFactory.php

class DBFactory {

    private $_dbh;

    public function __construct(DB $db) {
        $this->_dbh = $db;
    }

    public function Create() {
        return $this->_dbh->Connect();
    }
}

Class DBInstance.php

class DBInstance {

    private static $_dbinstance;

    public function __construct(DBFactory $dbfactory) {

        if (!isset(self::$_dbinstance)) {
            self::$_dbinstance = $dbfactory->Create();
        }
    }

    public function getDbInstance() {
        return self::$_dbinstance;
    }
}

Answer 1

Your code seems to do what you want it to.. but maybe we can use less object instantiation using inheritance and maybe we can avoid static properties in instanciated classes.

Also in regard to using a pattern of dependency injection that is able to handle multiple connections, but support using a single instance of it. exemple first, classes after

$params = array
 ('host'=>'localhost',
  'db'=>'ice',
  'user'=>'kopitar',
  'pass'=>'topnet',
  'charset'=>'utf8'); // passing the charset explicitely is great

$handle = new handle($params);
$db = $handle->getInstance();

we can either pass the $db to our functions

$location_names = Location::getLocationNames($db); 

or the whole $handle. as long as $handle is not reconstructed, it will always return the same database connection.

$location_names = Location::getLocationNames($handle);

if I want to reconstruct I need the whole $handle

$handle->__construct(/* params but with another database infos */);
$db2 = $handle->getInstance();

As for the classes, I think we want the params to arrive from the instanciated class, so we can change them later.

class db {
  function __construct($params) {
    foreach ($params as $param => $value) {
      $this->{$param} = $value; // assigns the connections infos
    }
  }
  protected function connect() {
    $dsn = 'mysql:host='.$this->host.';dbname='.$this->db.';charset='.$this->charset;
    return new PDO($dsn,$this->user,$this->pass);
  }
}

the factory creates a connection from params and passes it to something else, good factory

class factory extends db {
  protected function create() {
    return $this->connect();
  }
}

now we want to have our object to keep it's connection as long as we do not rebuild it. so we give it to instance

class instance extends factory {
  function instantiate() {
    $this->instance = $this->create();
  }
}

and last but not least, our handle which returns the instance. it could be in instance class.....................

but I feel like having four and find no real reason not to.

class handle extends instance {
  function __construct($params) {
    db::__construct($params);
    $this->instantiate(); // when we construct a handle, we assign an instance to the instance property
  }
  function getInstance() {
    return $this->instance;
  }
}

Answer 2

Why do you have a factory if you have a singleton? This is needless.

This is a never-ending debate, but I'm advocate of do not use singletons for database connections.

As far as in most applications, you have only one data channel, you can consider your database connection unique, but this might not be always true.

In deed, the effort made to create a singleton database connection is even bigger than just create a regular one.

Also, your class DB is not configurable, therefore, you need to change it when your connection parameters change. And I think DB is a very bad name for this.

I'd rather call this Storage and do something like:

inteface Storage {
    public function insert($container, array $data);
    public function update($container, array $data, $where);
    public function delete($container, $where);
    public function getAll($container);
    public function getOne($identifier);
}

final class PdoStorage implements Storage {
    private $dbh;
    private $dsn;
    private $user;
    private $pswd;

    public function __construct($dsn, $user, $pswd) {
        $this->dsn = $dsn;
        $this->user = $user;
        $this->pswd = $pswd;
    }

    // Lazy Initialization
    private function connect() {
        if ($this->dbh === null) 
            $this->dbh = new PDO($this->dsn, $this->user, $this->pswd);
    }

    public function insert($container, array $data) {
        $this->connect();

        // ... omitted for brevity
    }
}

Now, when you need a database storage, you do:

$someObj = new SomeClass(new PdoStorage(...));

Now you might be wondering if you will need to create an PdoStorage for each single object that depends on it.

The answer is: no!

Now you can use a factory to simplify your life.

class SomeFactory {
    private $defaultStorage;

    public function __construct(Storage $storage) {
        $this->defaultStorage = $storage;
    }

    public function create($type) {
        // Somehow fetches the correct class to instantiate and put it into $class variable , for example... and then

        return new $class($this->defaultStorage); // Or you'd better do this with reflection
    }
}

$factory = new SomeFactory(new PdoStorage(...));
$factory->create('SomeClass');

This way, you can have just one database connector or more if you need.

Answer 3

KISS

Don't make things more complex than they are, of course this is just my opinion, but as I see it you are building a complex solution for a problem that someone else says might exist is some cases. Php is not multi threaded so there goes one of the biggest arguments overboard. (in very rare-occasions it might be)

I'm using singletons for my database connections for about 15 years now and never ever had a problem with them, I do play around with different connections having one singleton handle several connection instances, but whatever... it works great and everyone that looks at the code.. understands it directly. I'm not using globals because they can be overwritten and are kind of hard to predict (when it holds the correct object, and when/why they don't)

Use OOP to make your code cleaner, easier to work with and more flexible. Don't use it to fix problems that aren't there and make your code more complex because others tell you to.

An very simple example of a db-connection singleton class handling several different connections.

class singleton{
  private static $_instances=array();

  public static function getInstance($connectionName){
     if(!isset(self::$_instance[$connectionName]){
        self::$_instance[$connectionName]=self::_getConnection($connectionName);
     }
     return self::$_instance[$connectionName];
  }

}

just my 2 cents