Today I saw some code that looked a little like this:

<?php
class GreetingClass {
    private $de, $en, $fr;

    public function __construct(Deutsch $de, English $en, Francais $fr)
    {
        $this->de = $de;
        $this->en = $en;
        $this->fr = $fr;
    }
}

On the face of it, this is excellent. The class constructor is telling me EXACTLY what classes it depends on. I can see that it requires an instance of Deutsch, English and Francais, so I can ensure that I pass each of these when I instantiate this class, and my object then has all of its dependencies ready.

Only, the constructor here only revealed part of the story. When I look further down the code I can see that the methods that actually use these objects that I passed in through the constructor, will only use one of the three objects that I passed in at a time:

<?php
class GreetingClass {
    private $de, $en, $fr;

    public function __construct(Deutsch $de, English $en, Francais $fr)
    {
        $this->de = $de;
        $this->en = $en;
        $this->fr = $fr;
    }

    public function hello($lang)
    {
        switch ($lang) {
            case 'de':
                $hello = $this->x->hallo();
                break;
            case 'en':
                $hello = $this->y->hello();
                break;
            case 'fr':
                $hello = $this->z->bonjour();
                break;
            default:
                throw new InvalidArgumentException("Unsupported language: ".$lang);
        }
        return $hello;
    }
}

When I actually investigate the implementation, I find that although it could support all three, it would actually only ever call one of them.

I couldn't have known this without looking through the code, but the point here is that the constructor has unnecessary requirements. There is no actual need to pass in a Deutsch, an English and a Francais into this specific object.

In this particular case, the code was part of a Laravel application. Laravel has a very clever Dependency Injection Container at its heart, and when you try to instantiate a new class, as long as you've type hinted arguments adequately, it will attempt to supply all of the classes dependencies. In this case, it would have instantiated a new Deutsch, a new English and a new French, regardless of whether it was going to use one or not. If it already had one (and assuming it had been instantiated as a singleton) it would have given us the same object that it was using elsewhere.

Because we don't actually need all three, however, it didn't make any sense to provide all three to the constructor. So the code was rewritten to be more like this:

<?php
class GreetingClass {
    private $app;

    public function __construct(Application $app)
    {
        $this->app = $app;
    }

    public function hello($lang)
    {
        switch ($lang) {
            case 'de':
                $foo = $this->app->make("de");
                return $foo->hallo();
            case 'en':
                $foo = $this->app->make("en");
                return $foo->hello();
            case 'fr':
                $foo = $this->app->make("fr");
                return $foo->bonjour();
            default:
                throw new InvalidArgumentException("Unsupported language: ".$lang);
        }
    }
}

This is an improvement in that we now only have a single dependency, and we won't create any objects that we don't actually need.

So we're done here, right?

I disagree.

I now no longer have any idea what this object is doing. I'm passing it a miscellaneous DI container and it performs all sorts of magic that I cannot possibly understand.

A class constructor, in this case in PHP, but the same applies in any object-oriented language, is the gateway to your object. It should tell you what other classes it depends upon. It makes debugging much easier, and it makes porting the code much easier as well, even if we rarely port application code to a new framework.

There is no way that I can tell anything about how this class operates without reading through the rest of the code. This is a toy example which is only a handful of lines of code long, but in reality the class was considerably larger.

So what is the solution?

It's an oft-repeated mantra: Code to an interface, not an implementation.

Consider this code:

<?php
/**
 * First, define a common interface that all classes will implement
 */
interface Language {
    public function hello();
}

/**
 * Now declare Deutsch, which implements this interface
 */
class Deutsch implements Language {
    public function hello(){
        return "hallo";
    }
}

/**
 * Now English, the same interface, see?
 */
class English implements Language {
    public function hello(){
        return "hello";
    }
}

/**
 * And then Francais, again with the same interface
 */
class Francais implements Language {
    public function hello(){
        return "bonjour";
    }
}

So, what have we got here? Firstly, we've declared an interface (Language) which has only a single method; hello(). As with any interface we're only defining a method signature, so the name and any arguments (where applicable). There is no functionality. All objects that implement this interface MUST implement this method. It's our contract.

Next, we define our three classes; Deutsch, English and Francais. Note that all three implement the Language interface. Accordingly, all three have a hello() method. They have to! Notice that although each has the same method, each actually does something a little different, ie returns the localised version of the greeting "hello". Here we're implementing polymorphism, which is another post all in itself, but the point is that by implementing this interface we can now make our GreetingClass both easier to read, and easier to maintain.

Code to an interface, not an implementation. Remember?

So now we just need to update our GreetingClass example to use this code:

<?php
class GreetingClass{
    private $language;

    public function __construct(Language $language)
    {
        $this->language = $language;
    }

    public function hello()
    {
        return $this->language->hello();
    }
}

And that's it.

Now I can see quite clearly from my constructor that I need to provide a single object which implements the interface Language, and nothing else. I also no longer need to provide an argument to the hello() method, because the switch has gone. It's not needed any more, because there are no decisions remaining to be made; we made it in the constructor.

I no longer need to care about how the rest of the class works, as long as I know that once I've successfully instantiated this object that I have a hello() method which I can call. I don't need to know anything else.

My implementation would now look something like this:

<?php
$greeter = new GreetingClass(new Francais);
echo $greeter->hello(); // Prints "bonjour"

The point here is that I'm not needing to pass in anything that my class doesn't need. I'm also making it easy for me (and any other developers who may look at this code later) to understand what is happening in GreetingClass. In removing the switch I've also made maintenance easier, so if I want to add a new language, let's say Polski, I only need to implement the Language interface and pass it in to GreetingClass and I know that it will work:

<?php
/**
 * New Polski class, implementing the Language interface
 */
class Polski implements Language {
    public function hello(){
        return "halo";
    }
}

$greeter = new GreetingClass(new Polski);
echo $greeter->hello(); // Prints "halo"

Passing faceless DI containers around makes this much more difficult. I can't understand what is happening unless I dig through the code, and I can't just add a new class without revisiting GreetingClass and adding a new condition to my switch to support the new language class.

I'm sure not everyone will agree with me, but do let me know your thoughts in the comments. I'll certainly be advising all of my team to follow this example.

Bootnotes

A couple of points that we should mention here:

Unit Testing

If the original class had unit tests, then they would probably now fail, because the constructor now has different requirements, and the hello() method no longer behaves differently depending on the language that it is passed as an argument (it no longer has any arguments.

You would need to take this into consideration if you were refactoring your code in a similar fashion. The point that I'm trying to highlight here is to do it right in the first place, because it's much easier to maintain then, but if you are refactoring old code then you will have to update your unit tests, as well as looking for anywhere in your application where you have implemented these classes and ensure that these are updated too. Depending on the size of your application this may not be as big of a job as you think.

Polymorphism

We have touched on Polymorphism here. If you would like to read more, please refer to Polymorphism in PHP to see another example of coding to an interface, rather than an implementation. If you're interested in the more traditional subtyping polymorphism in PHP, read Polymorphism in PHP through Object Inheritance