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

PHP PDO Factory Classes

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

Problem

So, my question is more of a 'best practices' question rather than a question with a particular aim. This is my current understanding of PHP factories and how to incorporate them into a project using PDOs and dependency injection. I'm pretty new at this and I still don't understand a lot about this subject. As such, I really don't know if I'm doing this quite right. Could someone point me in the right direction?

 $val) {
                $settings[$key] = $val;
            }
        }
        // Return setings.
        return $settings;
    }
}

$settings = PDOSettings_Factory::build();

class PDOConnection_Factory {
    private function __construct() {}
    public static function build($args) {
        // Set up PDO connection
        if(!isset($args)) {
            return NULL;
        } else {
            $statement = 
                $args['dbType'].':'.
                'host='.$args['host'].';'.
                'dbname='.$args['dbName'];
            try {
                $dbh = new PDO(
                    $statement, 
                    $args['user'], 
                    $args['pass']);
                $dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
                // Return DBH
                return $dbh;
            } catch (PDOException $e) {
                echo 'Error: ',  $e->getMessage(), "\n";
            }
        }
        return NULL;
    }
}


And how to use the factories:

```
class demo {
private $dbh = NULL;
public function set($args) {
if (!isset($args)) {return FALSE;}
else {
$this->dbh = $args;
return TRUE;
}
}
public function __construct($args = NULL) {
$this->set($args);
}

public function doSomething() {
// Set SQL command here.
$sql = 'SELECT * FROM test';

$results = $this->dbh->prepare($sql);
// Perform param binds here.
$results->execute();
return $results;
}
}

$db = PDOConnectio

Solution

Formatting strings

You should try to avoid those kind of things:

$statement = $args['dbType'].':'.'host='.$args['host'].';'.'dbname='.$args['dbName'];


It's easy to miss something, and it's also this kind of constructions that leads to difficult to read code and (in other contexts) SQL injections. I would argue that using sprintf is a better idea here:

$format = '%s:host=%s:dbname=%s';
echo sprintf($format, $args['dbType'], $args['host'], $args['dbName']);


I don't know if it is considered idiomatic PHP or not, but it seems more readable and less error-prone to me.

PDO and exceptions

mnhg already gave you a way to get rid of the first return NULL in PDOConnection_Factory::build(), but it's not enough, you should also remove the second one. I assume it comes from the possible exceptions in PDO::__construct and PDO::setAttribute. The issue is that you're catching the exception but doing nothing with them.

You should instead embrace exceptions. :) A good first step is the setAttribute call. Since there's nothing you can do to handle them in the factory: you should instead let the application code decide what to do if something fails: for example, display a warning message to users and send an email to the administrator.

Code Snippets

$statement = $args['dbType'].':'.'host='.$args['host'].';'.'dbname='.$args['dbName'];
$format = '%s:host=%s:dbname=%s';
echo sprintf($format, $args['dbType'], $args['host'], $args['dbName']);

Context

StackExchange Code Review Q#21555, answer score: 2

Revisions (0)

No revisions yet.