1

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");

  }

}
tereško
  • 56,151
  • 24
  • 92
  • 147
krist4l
  • 15
  • 1
  • 4
  • I would go for the first option because I think you should only have one connection to the same database per request. – Jerodev Oct 06 '14 at 14:57
  • make sure that the instance u passs is single only when u are talking about databases... – Daniel Adenew Oct 06 '14 at 14:57
  • 1
    First of all, "model" is **not** a class. As for how to share DB connection see *[this post](http://stackoverflow.com/a/11369679/727208)* – tereško Oct 06 '14 at 15:15
  • having a separate db connection for each request is amongst the worst ideas you can get. You want a single connection used by all requests. You have two main avenue: implementing a singleton global static object, or inject the database connection in each class which needs it – Félix Gagnon-Grenier Oct 06 '14 at 16:56
  • 1
    @FélixGagnon-Grenier, May I see an example of the singleton global static object, please? – Monica Oct 18 '14 at 22:03

2 Answers2

4

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.

Community
  • 1
  • 1
Félix Gagnon-Grenier
  • 7,344
  • 10
  • 45
  • 58
-3

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:

  1. 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.

  2. 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.

  3. 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.

  4. 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.

  5. (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.

  6. 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.

Community
  • 1
  • 1
ItalyPaleAle
  • 6,678
  • 6
  • 41
  • 65
  • 2
    -1: recommendation to use anti-pattern, abuse of PDO class and vulnerability to SQL injections. – tereško Oct 06 '14 at 15:15
  • 1. Why would that be vulnerable to SQL injections? I never told him not to use prepared statements 2. Who said using singletons is wrong? and 3. Why is that abusing the PDO class? That's called creating another abstraction layer. @tereško – ItalyPaleAle Oct 06 '14 at 15:18
  • @tereško which "incorrect configuration"? there's no configuration here! -.-" and again, why would a singleton be wrong? it makes total sense here. And in my full class I add methods for example to retrieve the data I want (select common data from certain tables). He's free to do what he wants to. – ItalyPaleAle Oct 06 '14 at 15:23
  • @tereško I've been thinking and doing lots of research. Honestly, I cannot find any practical, non-academic reason for why a singleton is wrong in this case. "It makes it harder to have more than 1 db connection" -> PDO can connect to pools, and actually another DBAL can make it easier. That and all other arguments I read reported precise academic reasons, but to me just demonstrated dogmatism and narrow-thinking. One answer particularly struck me: http://stackoverflow.com/a/13183408/192024 I won't convince you that singletons are the best practice, but it's not a bad one IN PRACTICAL USE. – ItalyPaleAle Oct 06 '14 at 15:37
  • 1
    So you think that "cannot be unit-tested" and "hardcoded to a class name" are *academic* reasons. More power to you. Also, you should read the other answers in that post which you linked to. – tereško Oct 06 '14 at 15:43
  • @tereško if you don't prevent creation of other classes (notice I didn't overwrite the __construct() method) you can do all unit testing you want (now argue with me because I didn't follow the *proper singleton design pattern*?). And what does "hardcoded to a class name" mean, and why would it be an issue? Worst case: run a find&replace: any editor can do that in multiple files at once, and it takes *a few long seconds*... Look at advantages, instead: easy, quick and convenient access to your database, always. – ItalyPaleAle Oct 06 '14 at 15:49
  • @Qualcuno from SO's info tab on the singleton tab: `Singletons are arguably the most well-known, the most used, the most abused, and the most disputed design pattern in existence, frequently leading to heated discussions between their proponents and opponents.` lots of people hate singletons, with very valid reasons, as much as other people like it, for also valid reasons – Félix Gagnon-Grenier Oct 06 '14 at 16:59
  • @FélixGagnon-Grenier "Disputed" is the keyword. So if you feel like you've a better solution without using singletons, please feel free to post your answer. The OP will evaluate which solution is more convenient to him. But I refuse to think singletons are "wrong" because of all academic arguments I'm reading. So far, haven't found a single practical reason for not having a PDO (and *just PDO*) singleton. – ItalyPaleAle Oct 06 '14 at 17:03
  • @Qualcuno oh please do not worry, I myself use singletons with much contempt, I'm merely trying to get to you that singletons are disputed, and that even though you may not see why it is so, maybe you're not working in an environment where the flaws of singletons are evident. For exemple, what if you have to connect to multiple databases in your script, how would you manage your different connections if your connection manager can't have two connections at the same time (ie like a singleton?) – Félix Gagnon-Grenier Oct 06 '14 at 17:05
  • @FélixGagnon-Grenier I can confidently say that 99.9% of the programmers do not need to connect to more than one SQL database at once (meaning different databases, not part of the same pool). And the remaining 0.1% have probably more serious problems than a singleton. I do code thinking about scalability, but your example is too much of an edge case. (If you connect to other database, such as NoSQL/Cache, then again this code is not an issue, actually a single DBAL can make things easier). – ItalyPaleAle Oct 06 '14 at 17:09