3

In PHP, I have two classes: Database and Item.

Database contains the connection properties and methods. For example, Database::Query can be used to pass a query string, etc.

Item is a generic item identifier. It's built by passing it an itemID which is then used to query the database for the rest of the item information.

In this case, what's the best practice for creating Item objects if they require database access? Is it normal to create each one using this syntax:

$item = new Item(12345, $db);

Or is it better, acceptible, or possible to create the Database object and have it used for each Item created in the application, such that the call could become:

$item = new Item(12345);

The second seems a lot cleaner (and can be expanded so that similar types of objects don't also need that $db addon), but I'm looking for suggestions from those who have more experience at this than I do! Thanks!

Paka
  • 33
  • 3
  • 4
    You could use [this approach](http://stackoverflow.com/a/11369679/727208) with lazy initialization for DB instance (it also can be adjusted for mysqli). – tereško Dec 11 '13 at 18:28

3 Answers3

2

I would suggest that most seasoned developers would lean toward the approach of dependency injection as demonstrated in your first example.

Why?

Well largely because this allows you to decouple the class to which the dependency is being injected from the dependency's implementation.

So consider this dependency injection example:

Class some_class {
    protected $db;
    __construct($db) {
        if($db instanceof some_db_class === false) {
            throw new Exception('Improper parameter passed.');
        }
        $this->db = $db;
    } 

}

Here you could pass any type of object so long as it was an instance of some_db_class it could be a subclass of that object that implements the same methods used by this class. That doesn't matter to this class as long as the methods are implemented (you of course could also check that a passed object implements a specific interface in addition to or in lieu of checking its instance type).

This means that, for example, you can pass a mock DB object for testing or something like that. The class doesn't care as long as the methods are implemented.

Now consider the singleton approach (or similar instantiation of DB from with the class):

Class some_class {
    protected $db;
    __construct() {
        $this->db = some_db_class::get_instance();
    } 
}

Here you have tightly coupled your class to a specific database class. If you wanted to test this class with a mock DB implementation it becomes very painful in that you need to modify the class to do so.

I won't even get into discussion of using global as that is just poor practice and should not be considered at all.

Mike Brant
  • 66,858
  • 9
  • 86
  • 97
  • I would recommend merely typehinting for some_db_class, yet if you were to do it that way, there's an exception just for that: `InvalidArgumentException` ;) – Jimbo Dec 11 '13 at 18:48
  • @jimbo Yes type hinting is a good suggestion, though not helpful if you need to check that a passed object implements a specific interface. – Mike Brant Dec 11 '13 at 18:50
  • You can typehint for an interface of course... it's the best way of doing things :-) – Jimbo Dec 11 '13 at 18:51
-3

I would recommend using the Singleton Pattern for your database connection. This is actually the best practice. As you really dont need to instances of your database connection.

class Database_Instance
{
    private static $database;
    public static function getDatabaseObject() {
        if (!self::$db) {
            self::$db = new PDO( );
            }
        return self::$db;
    }
}

function callWhatSoEver()
{
    $connection = Database_Instance::getDatabaseObject();

}

For more information about the singleton pattern, see: http://en.wikipedia.org/wiki/Singleton_pattern

Kevin Goedecke
  • 940
  • 9
  • 24
-4

Typically a database connection object is global, or accessible globally. That works well for the vast majority of applications.

I do something like this (simplified for example purposes):

$db = connect_to_db();

function GetDB()
{
   global $db;
   return $db
}

//inside the item object

function Load( $id)
{    
   $db = GetDB();
   $db->query(..);
}

There are, of course, cases where this isn't the best route. As always, it depend on the specific needs of your application.

GrandmasterB
  • 3,220
  • 1
  • 20
  • 21
  • 3
    -1: **typically** that is the most harmful thing to do in ANY application. – tereško Dec 11 '13 at 18:30
  • No. Don't use global declarations. – Mike Brant Dec 11 '13 at 18:47
  • Oh please, for the vast majority of applications this works just fine. – GrandmasterB Dec 11 '13 at 19:06
  • @GrandmasterB Sure it "works", but no one should be passing this off as a suggested approach to any programming problem. You are requiring a class to understand some specific variable name that exists in global scope. This goes strictly against the OOP principal of encapsulation. Also what happens when you change that variable name? All of a sudden you have to go change a bunch of classes to update this variable name. You are tightly coupling your class to application details it has no business understanding. – Mike Brant Dec 11 '13 at 21:24
  • I believe you are misreading the example then. $db in the Load() method is local. GetDB() in the example, is a place holder for a globally available method or function that returns the instance of the connection object. The local var isn't tied to a name. The only thing the object has to be aware of is the name of the method used to return a reference to the db connection. This is far simpler to manage than needing to pass references to a db connection around for the creation of every object used in a system. – GrandmasterB Dec 11 '13 at 21:32
  • @GrandmasterB I get it. What you have done is basically a singleton pattern implemented via a function, with the function using a global declaration. The OP would be better off dealing with a class-based singleton if he didn't want to utilize dependency injection. As then, he would at least not be polluting the global namespace with various variables and functions. Besides requiring the class to have knowledge on how to instantiate a database object, when you start looking at developing quality, this approach does not lend itself to unit testing. – Mike Brant Dec 11 '13 at 22:38