What is better way for db connections in relevant instance, when using OOP approch in PHP?
- db connection passed to all instances
- each instance to have its own db connection?
Here is my code example.
Passing connection to constructor:
<?php
class Model {
public $db;
public function __construct($db) {
$this->db = $db;
}
}
Connecting to db in model constructor.
class Model {
protected $db;
public function __construct() {
$this->db = new Detabase_Con();
}
}
Users model
class Users extends Model {
public function showUsers() {
$this->db->select("SELECT * FROM users");
}
}
Singleton database access
This is pretty much disputed but ensures that you database connection won't be duplicated and is easier to use in existing code.
class DB {
private $instance=null;
private static function instance() {
if (self::$instance === null) {
self::$instance = new PDO(/*connection foo*/);
}
return self::$instance;
}
public static function __callStatic($method, $args) {
return call_user_func_array(array(self::instance(), $method), $args);
}
}
include('DB.php'); // the file containing the class
$sth = DB::prepare("SELECT foo");
$sth->execute();
$row = $sth->fetch();
This draws heavily from the simple PDO wrapper proposed in this answer. The wrapper was written for precise needs, so use it as is.
Dependency injection
This can mean that you have to think better about your code and maybe rewrite huge parts of it. The point is to type-hint your connection in your classes/functions signatures to make it clear it depends on PDO:
class user{
function construct(PDO $db,$user_id) {
// define your user with the infos from $db
}
}
$db = new PDO(/*connection foo*/);
$wants_db = new user($db,$user_id);
If you rewrite all your functions signatures to pass $db
, you may find that it is not better, and it might be true. You may have to redesign parts of your application so that not all the functions or methods depend on the database connection.
How to choose
The singleton approach is easier now, might suit your needs but but present problems down the road. You do not want to start this and refactor your code if you realize that passing it as a parameter was a better approach
The dependency injection compliant approach is harder now as it implies changes in your code, but this energy will mean that you will have better/cleaner/easier to maintain code eventually.
After having experimented with the singleton I think that implementing a injected database access is better. I think it represents a part of the SOLID thing. I'd say this makes for a better more future proof approach to database access in php oop scripts.
It's a really bad idea to have all your classes inherit from the one containing a database connection. That's not a problem with MVC per se, but rather is a misuse of the object-oriented programming approach.
Thus, the first approach looks MUCH better.
Personally, I like having my database connection as a singleton. For example:
<?php
class MyDatabase extends PDO
{
public static function SharedInstance() {
static $instance = false;
if(!$instance) {
if(!($instance = new MyDatabase('mysql:.....', '', '', array(PDO::ATTR_PERSISTENT => true)))) {
throw new Exception('Cannot connect to database');
}
$instance->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
$instance->exec('SET NAMES utf8'); // MySQL only
}
return $instance;
}
// ... More methods and utils for the database class, if you need to extend it
}
?>
Then, everywhere in the code (in any scope) I can recall the connection to the database by simply doing:
$db = MyDatabase::SharedInstance();
// And use it as a normal PDO object. For example:
$db->exec('some query');
You don't need to use PDO; you can do that also with MySQLi and other database drivers. But it works best when the database driver has an object-oriented interface.
PS: Just to make sure, always use prepared statements. The call to $db->exec
above is just an example of exposing PDO's interface.
In defence of my idea
I know singletons are disputed, and I'm not saying they are the best solution to any problem. However, in this case they CAN and DO make sense. Here's some of criticism I've received in the comments:
They make unit testing harder
Since I did not overwrite the __construct()
method to prevent initialization of other objects from the MyDatabase
class, you should be able to write tests without any issue.
You are hardcoding a class name
Suppose we were not using a singleton. Many users would use a global variable for the database connection, which is still "something hardcoded". Anyways, programming is hardcoding stuff, so I don't see why this should be an issue at all. Worst case: just run a find & replace on all your project files. Any editor can do that easily.
It's harder to connect to more than one (SQL) database
First of all, I bet only 0.01% of developers on SO actually need to connect to multiple SQL databases in the same PHP application. And for those who do, singletons are probably the last problem they're dealing with.
Thus said, with this approach you can still connect to NoSQL databases and caches, and actually you could include some caching logic in the MyDatabase class. If you need to connect to another SQL database using PDO, then just create another instance of the PDO class (nothing can stop you from doing that!), or create another singleton. Again, this is a non-issue.
You are allowing SQL injections
Since this is only a wrapper around PDO, I would say that PDO allows you SQL injections in the first place. If you don't know how to use prepared statements or quote your query parameters properly, then it's not my fault.
(You're allowing SQL injections) because it's poorly configured
I don't see any configuration in the code above. There's just a connection to a database using PDO.
It's bad because this SO post says so
Quote from Wikipedia:
Ipse dixit, Latin for "He, himself, said it," is a term used to identify and describe a sort of arbitrary dogmatic statement which the speaker expects the listener to accept as valid.
The fallacy of defending a proposition by baldly asserting that it is "just how it is" distorts the argument by opting out of it entirely: the claimant declares an issue to be intrinsic, and not changeable.
Other posts on SO, however, defend singletons too. However, at the end what's important is too keep an open mind, and not just barricate yourself behind walls of dogmatism.