patternphpMinor
PHP PDO Factory Classes
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?
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
$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:
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
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
You should instead embrace exceptions. :) A good first step is the
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.