HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

PDO class for multiple databases

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
databasesformultipleclasspdo

Problem

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() . "";
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() . "";
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_NUL

Solution

A quick recap from SO

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

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

Code Snippets

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

Context

StackExchange Code Review Q#30689, answer score: 4

Revisions (0)

No revisions yet.