4

I have a class that deals with a number of database operations. I want the class to be as re-usable and well-designed as possible, and I'm fairly new to OOP, so I would appreciate a solution to this:

Is it better practice to do:

class MyDatabase extends Database 
{
    private $connection;

    public function __construct(mysqli $connection)
    {
        $this->connection=$connection;        
    }

    //More functions below

}

or

class MyDatabase extends Database 
{
    private $connection;

    public function __construct()
    {
        $this->connection=new mysqli(...);
    }

    //More functions below

}

What are the pros/cons of both, and which one is used more often? I can't really decide for myself which one I should start using, and it will affect the rest of the application I'm writing.

Thank you

r.bilgil
  • 889
  • 2
  • 10
  • 15
  • 4
    First one. Read this post http://ralphschindler.com/2012/03/09/php-constructor-best-practices-and-the-prototype-pattern – elclanrs Sep 15 '13 at 23:32
  • 1
    I'd have to agree that the first method is better. The second method makes a new connection for every instance of the object. However, it may not be a bad idea to add `private function connecToDB()` that creates the database connection and is called if no DB object is passed to the constructor. –  Sep 15 '13 at 23:40
  • I guess it makes sense to not create a separate connection for every object. Would you suggest creating, say, a "DatabaseConnection" class which creates the connection, and then pass this to MyDatabase, or create mysqli directly and pass that? – r.bilgil Sep 15 '13 at 23:50
  • @r.bilgil , definitely Nr.1, especially since creating a mysqli instance requires that pass in additional parameters. If you used second approach, you would end up violating [LoD](http://c2.com/cgi/wiki?LawOfDemeter) or depending on global state. Also, you might re-think the whole "Database class" idea. You already have a database classes (mysqli and pdo). Instead of creating another layer, you should inject the DB connection classes that will require it. You might find [this approach](http://stackoverflow.com/a/11369679/727208) a good idea and adapt it for use with MySQLi instead of PDO. – tereško Sep 15 '13 at 23:54
  • Probably just the database connection. I don't see the need to make a separate class that will likely only have one function to create a database connection when you can do it just as easily without. –  Sep 15 '13 at 23:54

2 Answers2

1

I'd suggest the 2nd approach, especially if you are new to OOP.

A key principle in OO is "information hiding", you want to hide as much information as possible within a class so that not to expose complexities to the outside.

The advantage of the 2nd approach is that it completely hides all the database operation details, as the constructor takes no parameter.

Sometimes the 1st approach is preferable in a larger and more complex system where your "MyDatabase" class needs to be able to handle different types of underlying database connections, and needs to perhaps take in different types of connection handles in its constructor. But that is a different design, in which we are exposing the database handle detail, and essentially include that as part of the API (aka contract in OO terms) to the outside world.

Another common OO practice is frequent refactoring. If you have a good IDE and keep a reasonably good programming practice, you can fairly easily refactor your code from simpler approach (#2) to a more complex one (#1) without too much trouble. So I say go ahead with #2, keep it simple, and refactor down the road when/if needed.

In terms of performance, I see little difference in the two approach, you can (and should) always use persistent connections anyway. So the object overhead should be minimal.

stevel
  • 720
  • 4
  • 7
  • 2
    Geez, guess my suggestion isn't too popular with folks. But I still think for someone "new to OOP", it's a good idea not to get overwhelmed with all the so called best practices all at once. Give the guy some time to get comfortable, and a few programs working well. I'm sure he/she will get more comfortable with things later on, like dependency injection, etc. (imaging explaining that to someone new...) – stevel Sep 16 '13 at 00:33
  • Thanks for your input, not sure why you got buried despite your clearly articulated and valid reasoning. I am indeed fairly new to OOP but in this case I can see the clear advantage of dependency injection, and after watching the videos and slides being thrown about I think it's the way to go for my application. – r.bilgil Sep 16 '13 at 22:24
1

I would suggest the first one, no hard coded dependency, testable and can change the connection object at anytime. I highly recommend you to watch this.

The Alpha
  • 143,660
  • 29
  • 287
  • 307
  • 1
    I would recommend to not link to that presentation. While it contains some good tidbits on dependency injection, it also shows a large number of bad practices, while implying that that is the proper way. – tereško Sep 16 '13 at 00:05
  • Maybe [this one](https://www.youtube.com/watch?v=IKD2-MAkXyQ). It's kinda hard to get any good and newbie-friendly materials on subject. It's not like you can throw [Fowler](http://martinfowler.com/articles/injection.html) at them and expect to understand =/ – tereško Sep 16 '13 at 00:11
  • 3
    I just read @tereško 's link and I can't spot the bad practices. What specifically is it suggesting that isn't good practice? – r.bilgil Sep 16 '13 at 00:12
  • 1
    @r.bilgil up to slide 34 everything is just fine. And the expression that I used was wrong. The problem with that presentation is the "DI container" part, which takes up ~60% of presentation (+10% of advertisement). The described implementation will actually result in a service locator with hidden global state. Also there is NO explanation how DI container has to be used, which is actually not intuitive for most people. Even if you DI containers is properly implemented, misuse will cause significant damage to a project. Bottom line - stop at slide 34. – tereško Sep 16 '13 at 00:42