Qafoo GmbH - passion for software quality
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

:Author: 		Tobias Schlitt
:Date:			Mon, 07 Oct 2013 11:25:46 +0200
:Revision: 		13
:Copyright:		All rights reserved


=================================
ContainerAware Considered Harmful
=================================

:Description:
    Making objects aware of the Dependency Injection Container can seriously
    harm the object oriented structure of your application.
:Keywords:
    php, dependency injection container, dic, inject, dependencies, object
    oriented design, testing
:Abstract:
    A while ago I `tweeted`__

        ContainerAware is the new Singleton.

    While many people agreed by retweeting and faving. I feel the need to
    elaborate some more on this statement and safe the explaination for the
    future.

__ https://twitter.com/tobySen/status/378780141826355200

A while ago I `tweeted`__

    ContainerAware is the new Singleton.

While many people agreed by retweeting and faving. I feel the need to elaborate
some more on this statement and safe the explaination for the future.

__ https://twitter.com/tobySen/status/378780141826355200

*TL;DR*: No class of your application (except for factories) should know about
the Dependency Injection Container (DIC).

----------
Background
----------

The ``ContainerAware`` interface  (actually ``ContainerAwareInterface``,
``ContainerAware`` is a basic implementation of it) is part of the Symfony2__
API__, but a similar concept is known from many other frameworks and many
applications rely on it. It defines only the one method ``setContainer()``, which allows to inject
the DIC into an object so that it can directly retrieve services from it.

__ http://symfony.com
__ http://api.symfony.com/2.0/Symfony/Component/DependencyInjection/ContainerAware.html

It is most common to have controllers implement this interface. In fact, the
`Symfony2 base controllers`__ do so and there is a `base class for shell
commands`__ that does it.

__ http://api.symfony.com/2.0/Symfony/Bundle/FrameworkBundle/Controller/Controller.html
__ http://api.symfony.com/2.0/Symfony/Bundle/FrameworkBundle/Command/ContainerAwareCommand.html

------
Issues
------

Accessing the DIC in your classes can seriously harm maintainability and code
re-usability in the long run.

Reduced Testability
===================

.. note::
    Get your team a boost with a hands-on Qafoo `workshop on object oriented
    design`__ (OOD) and `automated testing`__.

__ /services/training/topics/object_oriented_design.html
__ /services/training/topics/testing_with_phpunit.html

Unit testing is the most common automatic test method in the PHP world. If you
have the container injected into your objects, this becomes much harder. There
are three strategies to approach testing a class that gets the container
injected:

1. Mock the container and make it return the mocked service mocks,

2. Use the container in your test cases and make it return the mocks

3. Mock the subject of test to override the ``get()`` method that is commonly
   used to access services inside the class.

The first solution actually requires you to create a container mock that basically does
the same thing as the real container. Except for mocking overhead you
don't win much with this. So, if you are already in the situation that your
classes have a dependency to the DI container, you're better of with version 2.

If you choose the second variant, you're formerly not writing a unit test, but
an integration test instead. This is not a problem by default and integration
tests are important, especially if you use an external framework for
development. However, if you intend to write unit tests, you simply don't in
this case, since another class (the DIC) is put under test.

The third variant is a simple no-go. You should never mock methods of your
subject, because you cannot ensure that the mocked version of the code mimics
what really happens. Your test cases lose a big amount of the safety that they
should actually provide you with. Whenever you feel the need to mock a method of
the test subject, that is a clear sign for the need to refactor (so-called
*code smell*).

Hidden Dependencies
===================

If you go for a real dependency injection approach (my favorite is constructor
injection), you can easily see when a class does to much, just by looking at
its dependencies. Think about a constructor like this::

    <?php

    class UserProfileController
    {
        // ...

        public function __construct(
            UserRepository $userRepository,
            Search $search,
            MailGateway $mailGateway,
            Router $router,
            HttpClient $httpClient,
            PermissionService $permissions
        )
        {
            // ...
        }

        // ...
    }

You can easily spot that there is something fishy. The class receives quite
many dependencies and these do not fit very well together. Some are from the
infrastructure layer, others seem to be business services and even others
correlate with the MVC structure. If you see this constructor, it's eminent
that there is need for refactoring. You will note that at the latest when you
add another dependency to it.

Now compare that with the following code::

    class UserProfileController extends ContainerAware
    {
        // ...

        public function __construct()
        {
            // ...
        }

        // ...
    }

There is no way to see what the actually dependencies of the class are. In
order to see that, you need to go through the code and look for places where
the Dependency Injection Container is used to retrieve objects. In real life,
nobody will ever do that, especially if this is hidden behind
``$this->get('serviceName')`` calls.

Feature Sneak-In
================

This issue is a direct result of the previous one: If you can, you
eventually will. And by this I mean access whatever service you might think you
just need.

With access to the DIC a developer has the freedom to just grab any object and
use it. This might be convenient to implement hacks, if you don't know where
else to put a certain functionality when being in a rush. But on the other side
of the coin, there is nothing which forces you to clean up the mess.

A common result is, that more and more business logic is collected in the
controllers, making them become huge transaction scripts, which leads to code
duplication and hidden business rules.

----------
Conclusion
----------

Making your classes aware of the DIC is not an option. Even if you really feel
you need to, just don't do it. To be on the safe side I even recommend to `make
your controllers services`__. 

__ http://symfony.com/doc/current/cookbook/controller/service.html

If you develop Symfony2 based applications and run into the issue of having to
`inject too many framework services`__ into your controllers, my co-worker
Benjamin has a great post about that in his personal blog.

__ http://www.whitewashing.de/2013/06/27/extending_symfony2__controller_utilities.html

..
   Local Variables:
   mode: rst
   fill-column: 79
   End: 
   vim: et syn=rst tw=79


Trackbacks
==========


Comments
========