When Dependency Injection goes Wrong
Introduction
Dependency Injection (DI) is finally starting to take off in a big way in PHP. Whilst there have been projects such as Phemto for a while now it has taken it being made a fundamental part of, the now in beta, Symfony2 to really make its mark. Whilst you can still avoid it when writing Symfony2 application it is used throughout the framework components themselves and it provides an excellent Dependency Injection Container (DIC) to make using it application easy. In this post I am going to look at the advantages of using Dependency Injection and how to avoid missing out on these advantages whilst appearing to be using it.
Dependency Injection Advantages
Easier Testing
If you have a dependency in a class then it makes true Unit Testing difficult as you will be testing the functionality of the dependency when you actually want to test the functionality of the code using it.
In the following example we are using a Logger object to log a message:
<?php
namespace LimeThinking\SpringBundle\Blog;
use LimeThinking\SpringBundle\Logger\FileLogger;
class BlogPost
{
public function createPost()
{
//--
$logger = new FileLogger;
$logger->Log('action taken');
//--
}
}
In a unit test for this class the real logger will be instantiated and a log actually made. This is unwanted behaviour for several reasons. Firstly we are not testing the Logger but any bugs in it will break our test. A real log will be created, in this case in a file, interactions with things such as the file system, database etc. will slow tests making you less likely to run them. Checking whether the log method is called requires checking the actual log.
Much better is to inject the Logger object into the class:
<?php
namespace LimeThinking\SpringBundle\Blog;
class BlogPost
{
protected $logger;
public function __construct($logger)
{
$this->logger = $logger;
}
public function createPost()
{
//--
$this->logger->Log('action taken');
//--
}
}
In the test we can now inject a mock object instead. Any bugs in the real Logger will not break the test (they will hopefully though break the test for the FileLogger), the interaction with the file system will not take place and we can just ask the mock if its Log method was called. You can read more about using mock objects in PHPUnit tests in the PHPUnit manual.
Loosely coupled code
If you have a dependency instantiated directly in a class then you are tied to that implementation of the dependency. This means that if you want to swap to a different implementation you need to change the code. Perhaps creating a whole new version which uses a different dependency or introducing switch statements that check configuration to decide which to use, which can quickly lead to very messy code:
<?php
namespace LimeThinking\SpringBundle\Blog;
use LimeThinking\SpringBundle\Logger\FileLogger;
use LimeThinking\SpringBundle\Logger\SystemLogger;
use LimeThinking\SpringBundle\Logger\DatabaseLogger;
class BlogPost
{
public function createPost()
{
//--
$config = new Config();
$logType = $config->Get('logType');
switch($logType)
{
case: 'file':
$logger = new FileLogger;
break;
case: 'system':
$logger = new SystemLogger;
break;
case: 'database':
$logger = new DatabaseLogger;
break;
}
$logger->Log('action taken');
//--
}
}
Using Dependency Injection you just pass in the object you want to use and use it, the client code does not need to be involved in the decision of which to use at all.
Seems simple enough, what's all the fuss?
It is simple, it really is just passing dependencies in via the constructor or a method, so where does it go wrong? Well the first mistake would be to just shift the dependencies up to the next level of code. So the code that uses the object has to deal with its dependencies.
<?php
namespace LimeThinking\SpringBundle\Controller;
use LimeThinking\SpringBundle\Blog\BlogPost
class BlogController
{
public function createAction()
{
//--
$logger = new FileLogger;
$post = new BlogPost($logger);
$post->createPost();
//--
}
}
This has just shifted the problem elsewhere, so how do we deal with this? Well the problem is that we have not gone far enough, this class also needs its dependencies injecting. This will mean that it looks like this:
<?php
namespace LimeThinking\SpringBundle\Controller;
use LimeThinking\SpringBundle\Logger\FileLogger;
use LimeThinking\SpringBundle\Blog\BlogPost
class BlogController
{
protected $post;
public function __construct($post)
{
$this->post = $post;
}
public function createPost()
{
//--
$this->post->createPost();
//--
}
}
If we keep doing this we will end up at the top, at what ever the point of entry is. This is not a bad thing as we have now effectively separated the creation of our objects from their use. This mean that the choice of how to wire together the objects is done as they are executed but before hand. This means that we can now reassemble them for different applications that have different requirements without needing to touch the code itself.
Of course wiring up the objects now becomes a fairly big job which is where Dependency Injection Containers come in to play as they can ease the process of doing this. They are not however Dependency Injection but rather tools to help with its implementation. They are a fantastic tool when done right but they do cloud the issue rather and make DI seem more complicated than it is.
How to avoid the benefits of Dependency Injection
Poor Type Hinting
Using dependency injection goes hand in hand with using type hinting in my opinion. If you do not use type hinting at all on the injection points then you either need to check the type of the injected dependency before using it or risk running into unexpected errors. If the wrong dependency is passed in then methods called on it may not exist or worse a method with that name exists but calling has unfortunate results.
So type hinting will ensure that the error is thrown at the expected place, when the incorrect type of dependency is injected. Type hinting in this way is easy with constructor and setter injection (see my post Symfony2: Dependency Injection Types for details of these types of DI), with property injection you cannot do this and must rely on other checks rather than the simplicity of type hinting.
So if we just start using type hints then all is good? Well not necessarily, its still possible to lose the benefit of loosely coupled code by enforcing that a certain class has to be passed in:
<?php
namespace LimeThinking\SpringBundle\Blog;
use LimeThinking\SpringBundle\Logger\FileLogger;
class BlogPost
{
protected $logger;
public function __construct(FileLogger $logger)
{
$this->logger = $logger;
}
public function createPost()
{
//--
$this->logger->Log('action taken');
//--
}
}
If you type hint the name of a class then that needs to be passed in so changing to a different dependency is only possible by subclassing it. Much better is to type hint using interfaces, this way you can be sure that the dependencies has all the correct methods without being tied to any particular implementation:
<?php
namespace LimeThinking\SpringBundle\Blog;
use LimeThinking\SpringBundle\Logger\LoggerInterface;
class BlogPost
{
protected $logger;
public function __construct(LoggerInterface $logger)
{
$this->logger = $logger;
}
public function createPost()
{
//--
$this->logger->Log('action taken');
//--
}
}
Making the Container a Dependency
The advantages of DI are lost if you do not avoid making a DIC itself a dependency in your code. So how and why would you do that. How, is by requesting objects from the container directly. Why, is usually because of failing to push all your dependencies all the way out of the code. Often learning about DI with reference to a container is part of the problem as it makes you think DI is about getting things from a DIC when it is not at all. Doing this makes the container into a Service Locater and not a DIC at all. Especially if you make it a Singleton and request from it statically:
<?php
namespace LimeThinking\SpringBundle\Blog;
class BlogPost
{
public function createPost()
{
//--
$logger = DependencyInjectionContainer::get('logger');
$logger->Log('action taken');
//--
}
}
Not quite as bad but still problematic is injecting the container itself:
<?php
namespace LimeThinking\SpringBundle\Blog;
class BlogPost
{
protected $container;
public function __construct($container)
{
$this->container = $container;
}
public function createPost()
{
//--
$logger = $this->container->get('logger');
$logger->Log('action taken');
//--
}
}
So why is this bad? well to start with you now need to use the container in your tests or a mock container if you are injecting it. This leads to additional unnecessary code making tests harder to write in the first place and maintain, and let's face it hard to maintain tests don't get maintained.
This is not the only issue though and it is wrong to think of DI as only being about improving testability. If we inject the container and ask it for a particular service in multiple locations in our code then we are no longer as loosely coupled as we should be. Now all code requesting that service is tied to having one implementation of it.
Being Dependant on the Container
You can avoid any direct use of the DIC in your class and still fall into the trap of being dependant on it. This is by using one of the less common forms of DI, property injection. With property injection you inject the dependency by just setting a property of the object to it. It seems attractive at first, no need to write setter methods which often do nothing but set the property anyway. The whole point of setters is to avoid having public properties though and this is particularly important if you do not want the dependencies to be changed by any bit of code but set at the time of creation and left alone.
Some DICs though, such as Symfony2's allow you to avoid this problem by making the property protected or private. they do this by then using reflection when creating the objects allowing the property to be set despite not being public. So great, best of both worlds then? Unfortunately not, you have here introduced a more subtle dependency on the DIC, the object can no longer be directly instantiated and its dependencies set without using reflection. So it is now difficult to test as we cannot just create it and set its dependencies without using the container or reflection directly.
It is also now difficult to make use of this class in a small application where you may not need a heavy weight DIC if it used setter injection rather than property injection it could be created easily but now we need to use the DIC to create it. You are also tied to a particular DIC since any good one will support constructor and setter injection but not all will support property injection to non-public properties.
Tying dependency choice to class definition
It is easy to tie the choice of dependency to the class definition using interface injection: some DICs let you specify injecting dependencies via interfaces, this means that any class that has that interface will have the dependency injected into it. It is effectively a form of setter injection as the classes implementing the interface will have to have the setter method. The following example shows interface injection as done in an early version of Symfony2, it has now been removed because of the issues it raises:
<?php
namespace LimeThinking\SpringBundle\Blog;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
interface ControllerInterface
{
public function setResponse(Response $response);
public function setRequest(Request $request);
}
<?php
namespace LimeThinking\SpringBundle\Controller;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
class StaticController implements ControllerInterface
{
protected $response;
protected $request;
public function setResponse(Response $response)
{
$this->response = $response;
}
public function setRequest(Request $request)
{
$this->request = $request;
}
public function indexAction()
{
//do stuff to create response
return $this->response;
}
}
<?xml version="1.0" ?>
<container xmlns="http://symfony.com/schema/dic/services"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
<services>
<service id="lime_thinking_spring.response"
class="Symfony\Component\HttpFoundation\Response"/>
<service id="lime_thinking_spring.response"
class="Symfony\Component\HttpFoundation\Request"/>
<service id="lime_thinking_spring.static"
class="LimeThinking\SpringBundle\Controller\StaticController"/>
</services>
<interfaces>
<interface class="LimeThinking\SpringBundle\Controller\TestInterface">
<call method="setTemplating">
<argument type="service" id="templating" />
</call>
</interface>
</interfaces>
</container>
The problem is that you are not specifying the dependencies a class should have independently of it any more but in the class definition by implementing the interface. Whilst you may have separated the specifying the dependency to inject into the interface it is now injected to all classes implementing that interface which has introduced a tight coupling between all classes with that interface. If a particular application's wiring does not require a class to have that dependency then the interface needs to be removed from the class to avoid it receiving it which means making changes to the class itself which we should not have to do with DI.
Another way to tie the DI configuration to the class definition would be to use annotations in the comments to decide what dependency to inject. This is a bad way of doing the configuration as it now needs a different set of comments for any change in configuration.
Conclusion
Dependency Injection is a must if you want to create easily testable, loosely coupled code. However it is important to ensure that you avoid the above if you actual want to get these benefits. If you do not then you are still creating code that is hard to test and/or tightly coupled with the additional complexity of a DIC thrown in.