In my previous post I argued against setter injection. Optional dependencies are one of the main objections raised in the comments and elsewhere. I did mention these with a suggestion of just making them optional constructor arguments. I also mentioned that refactoring to stop it being optional was a solution.
I think that this is worth exploring further. I think that the disadvantages of setter injection means it should be avoided. I do not think any advantages for dealing with optional dependencies outweigh the disadvantages. This is not an opinion shared by all though.
Looking at how I deal with optional dependencies made me realise that I do not just avoid setter injection. I also usually avoid having optional dependencies. Having thought this through more will now avoid using them at all.
So why are optional dependencies a problem? Often an optional dependency is a sign that the class has several behaviours. So it has several responsibilities and is not adhering to the Single Responsibility Principle. A class with an optional dependency does one thing always. It does another thing sometimes if the optional dependency is present.
We would be better extracting the optional behaviour into another class. This new class could then have a single responsibility for that behaviour. In the new class this would not be optional and we can now choose which version. We are now deciding if we want the optional behaviour only in the config not in the config and in the object.
Even if this is not possible then we would still do well to avoid the optional dependency. Consider logging as the optional behaviour, which was a common counterexample. So a class with optional logging may look like this:
private $otherDependency;
private $logger;
public function __construct(OtherDependency $otherDependency, LoggerInterface $logger = null)
{
$this->otherDependency = $otherDependency;
$this->logger = $logger;
}
public function doSomething()
{
//do something
if ($this->logger) {
$this->logger->info('something to log');
}
}
There is another issue here which is that we have to wrap any use of the logger is a conditional. If we do not then we would get a fatal error when calling the non-existent logger's method. This adds complexity to the class and raises the risk of bugs. It is easy to avoid this bug with a spec or unit test. We could avoid even having to go to that effort though. Our class would be simpler if it looked like this:
private $otherDependency;
private $logger;
public function __construct(OtherDependency $otherDependency, LoggerInterface $logger)
{
$this->otherDependency = $otherDependency;
$this->logger = $logger;
}
public function doSomething()
{
//do something
$this->logger->info('something to log');
}
We can have this and still achieve the desired behaviour of optional logging by using a Null object. We can pass in a null logger that implements the logger interface but which does nothing when we do not want logging. Our class does not need to be aware that this is the case, it will use anything that has the interface. In fact if you are using a PSR3 logger then there is null logger implementation in the psr/log package.
Yes, we need an extra class but optional logging is something we will find in many classes. For the sake of a single extra class we can remove conditionals from a lot of places in our code. This reduces the number of potential bugs and makes the code more readable. Removing a lot of conditionals in exchange for creating a single class is a good refactoring of code.
It is no more complicated to turn logging on and off in configuration. When we do not need logging the logger service can just be the null logger. We will have turned off logging without making any other changes.
We now have simpler code. We can remove all the conditionals. We avoid setter injection. All without adding anything more complex to our code than a simple null implementation of the logger.
My thought is that optional dependencies are themselves a code smell. Setter injection is not a solution to this smell. Stopping the dependency being optional is more important than how we inject it.