1
\$\begingroup\$

I have a PDO class below:

class DB {

        private $dbh;
        private $stmt;

        static $db_type;
        static $connections;

        public function __construct($db, $id="") {

             switch($db) {
                case "db1":
                      try{

                          $this->dbh = new PDO("mysql:host=localhost;dbname=ms".$id, 'root', '', array( PDO::ATTR_PERSISTENT => true ));
                      }  catch(PDOException $e){
                          print "Error!: " . $e->getMessage() . "<br />";
                          die();
                      }
                break;
                case "db2":
                      try{
                          $this->dbh = new PDO("mysql:host=localhost;dbname=users", 'root', '', array( PDO::ATTR_PERSISTENT => true ));
                      } catch(PDOException $e){
                          print "Error!: " . $e->getMessage() . "<br />";
                          die();
                      }
                break;
            }
            self::$db_type = $db;
        }

        static function init($db_type = "", $id){ 

            if(!isset(self::$connections[$db_type])){ 
                self::$connections[$db_type] = new self($db_type, $id); 
            } 

            return self::$connections[$db_type];
        }

        public static function query($query) {

            self::$connections[self::$db_type]->stmt = self::$connections[self::$db_type]->dbh->prepare($query);
            return self::$connections[self::$db_type];
        }

        public function bind($pos, $value, $type = null) {

            if( is_null($type) ) {
                switch( true ) {
                    case is_int($value):
                        $type = PDO::PARAM_INT;
                        break;
                    case is_bool($value):
                        $type = PDO::PARAM_BOOL;
                        break;
                    case is_null($value):
                        $type = PDO::PARAM_NULL;
                        break;
                    default:
                        $type = PDO::PARAM_STR;
                }
            }

            self::$connections[self::$db_type]->stmt->bindValue($pos, $value, $type);
            return self::$connections[self::$db_type];
        }

        public function execute() {
            return self::$connections[self::$db_type]->stmt->execute();
        }
    }

Is it ok for multiple DB connections or not?

\$\endgroup\$
3
\$\begingroup\$

A quick recap from SO

  1. If you want to implement the Singleton pattern (first read about SOLID, though, and pay special attention to injection), make the constructor private.

  2. Your query method is static, why? it defaults (without the user being able to do anything about this) to the last connection that was established. Are you sure that's what the user wanted? Of course not! But then again, all of the other methods, like execute exhibit the same behaviour, so Everybody will end up working on the same connection, until they run into trouble and revert to using their own instances of PDO.

As ever, everything I have to say about custom wrapper classes around an API like PDO offers, can be read here. What I think of DB wrappers in general can be found here.

If you want to have all current connections available globally, then your code should shift towards a Factory, not a Singleton pattern:

class Factory
{
    private static $connections = array();
    public static function getDB($host, array $params)
    {
        if (!isset(self::connections[$host]))
        {
             self::connections[$host] = new DB($params);
        }
        return self::connections[$host];
    }
}
\$\endgroup\$
  • \$\begingroup\$ Do you know any code examples to look at? \$\endgroup\$ – user889349 Sep 2 '13 at 12:14
  • \$\begingroup\$ @user889349: check the links, there's a lot of info in them, including code examples. I've just quit smoking and have a lot of work to do, so I can't be bothered writing long answers today, sorry :s \$\endgroup\$ – Elias Van Ootegem Sep 2 '13 at 12:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.