Wednesday, November 18, 2009

To set or not to set

You probably know I am a test-infected developer and big proponent of Dependency Injection. You also have seen from the examples in this blog that I favor constructor injection, where a component asks for his collaborators in its own constructor:
class FacebookService
{
    private $_httpClient;
    public function __construct(HttpClient $client)
    {
        $this->_httpClient = $client;
    }
}
A factory or a configurable container can then recursively resolve dependencies and provide the class with what it asks for. The constructor's wiring code is dumb to write, but it is very concise and expresses intent: assigning the collaborator to a private property which will not be subsequently touched (as there are no setters).
The Api is also very clear as the constructor specifies everything is needed to compile and instantiate this class, while it does not provide the means to change the collaborators.

When the collaborators number is high, however, we may find difficult to use a constructor with a long signature. The obvious solution is trying to reduce coupling and analyzing the collaborators to see if everyone of them is really mandatory. It may be the case of a class that has too much responsibilities in accessing different parts of the object graph, or that collaborators leak into the class while they should be encapsulated in some other component.
Sometimes, there is nothing we can do to reduce the collaborators number:
class CommentsRepository
{
    private $_dbAdapter;
    private $_mailer;
    private $_logger;

    public function __construct(Zend_Db_Adapter $dbAdapter = null,
                                Zend_Mail_Transport_Abstract $mailer = null,
                                Logger $logger = null)
    {
        $this->_dbAdapter = $dbAdapter;
        $this->_mailer = $mailer;
        $this->_logger = $logger;
    }
}
This happens in some common cases:
  • the collaborators are options (value objects or scalars) which change the behavior of the component;
  • the class is a mediator between many objects and it is its own responsibility to deal with many collaborators.
Again, we can try to minimize the options or the objects involved, but the essential complexity will never vanish. Another solution might be passing the services via a method parameter only when they are used, but it usually violates encapsulation (a Controller having to keep a reference to a Logger even if it does not use it). Moreover, they may be needed in every method.
A long constructor is not clear as in almost any languages there are no named parameters (thanks Python) and we can forgot the parameters order in manual injection, or we may find difficult to extract automatically metadata on the collaborators if we are in a dynamic language like php.
The type of dependency injection we should adopt is slightly different: setter injection. This approach transforms the CommentsRepository class in:
class CommentsRepository
{
    private $_dbAdapter;
    private $_mailer;
    private $_logger;

    public function setDbAdapter(Zend_Db_Adapter $dbAdapter)
    {
        $this->_dbAdapter = $dbAdapter;
    }

    public function setMailer(Zend_Mail_Transport_Abstract $mailer)
    {
        $this->_mailer = $mailer;
    }

    public function setLogger(Logger $logger)
    {
        $this->_logger = $logger;
    }
}
Though, there are some problems with setter injection that we should solve:
  • setters allow changing collaborators after the construction: often it is a conterproductive operation and so it should be avoided. The setters can check if the corresponding private property is null before accepting the parameter.
  • the Api is not clear: why there are setters if I cannot set anything? I suggest to extract an interface where the setters are not present. This solves also the previous problem as the client class will depend only on an interface where setters are not defined and in static languages it is not even allowed to call them. In dynamic languages, the developer should refer to the Api documentation of CommentsRepositoryInterface and not of the CommentsRepository concrete class.
  • we may forgot a collaborator: both in manual and automated dependency injection you can forgot to call a setter or to add a collaborator to the configuration, and the result is a broken object hanging around. So you should maintain some form of test for the factory or the container (typically in integration tests). A missing collaborator is a wiring bug and it is simple to solve since it is going to manifest nearly always: the application will explode saying you called a method on null. Note that since I use null defaults for constructor parameters this problem is also present in constructor injection.
I hope you consider setter injection, as I avoided it without real reasons in the past and the design of your application can benefit from it.

5 comments:

Saulo Silva said...

Interesting article.

In general, I would rather sacrifice 'easiness of use' and have a constructor with a longer signature but that defines well the intent. However, I've seen classes whose constructor list of parameters was scary to say the least--it made the instantiation of an object really painful.

Giorgio said...

I delegate all instantiation to factories/builders/containers, and there are different ways to provide seams for injection. The important part is avoiding singletons. :)

Anonymous said...

Question: What advantage is there to specifying a null default parameter with a class type hint? Such as:

public function __construct(Zend_Db_Adapter $dbAdapter = null)

It is my understanding that if the default value is used, there will be a type-hinting error anyway ('null is not an object of type Zend_Db_Adapter' or something like that), so what advantage is there using a default null parameter here over no parameter?

Just trying to see what I'm missing. Thanks!

Giorgio said...

When mocking this class, with null defaults you have not to tell phpunit to override the constructor, nor specifying parameters:
$mock = $this->getMock('CommentsRepository');
versus
$mock = $this->getMock('CommentsRepository', array('methodToMock'), array(), '', false);
Otherwise the first call to getMock() in phpunit does not work: it tells you there are missing parameters.

Anonymous said...

i truthfully love all your posting way, very attractive.
don't quit as well as keep penning simply because it just truly worth to look through it,
looking forward to read more of your own web content, enjoy your day!

ShareThis