Dependency Injection Smells
In the previous article, I have discussed the difference between constructor and setter injections. Now, it’s time to pay attention to some examples of how not to use Dependency Injection.
Too many dependencies
When a class has a lot of dependencies, often in a constructor it is the first sign that this has does too much and has a lot of responsibilities (violates Single Responsibility Principle):
<?php
class OrderController
{
public function __construct(
OrdersRepository $ordersRepo,
PaymentGateway $payments,
ShippingService $shipping,
Logger $logger
Email $mailer
)
{
$this->ordersRepo = $ordersRepo;
$this->paymentGateway = $payments;
$this->shipping = $shipping;
$this->logger = $logger;
$this->mail = $mailer;
}
}At first, examine your constructor dependencies, maybe some of them are not required to be in a constructor and can be passed via method injection. For example, it is unlikely that we will need an instance of ShippingService to process payment. But this approach does not really solve the problem.
Another option will be to pass only a container to the constructor and later resolve all of these dependencies.
<?php
class OrderController
{
public function __construct(Container $container)
{
$this->ordersRepo = $container->get('ordersRepository');
$this->paymentGateway = $container->get('paymentGateway');
$this->shipping = $container->get('PaymentGateway');
$this->logger = $container->get('Logger');
$this->mail = $container->get('Email');
}
}But, in this context, it will be an anti-pattern Service Locator. Depending on a container hides all the information about class dependencies. If we want to test this class, we should examine the class and find out all the dependencies that it resolves out of the container and mock them for every test. We also cannot be sure that all the dependencies are already correctly registered in the container. In this way, we lose all advantages of Dependency Injection. Objects shouldn’t know about the container.
A better option is to break OrderController into several small classes, each with its own responsibility, like PaymentController or ShippingController.
Cyclic Dependencies
Class A depends on class B, but class B depends on A.
<?php
class A {
protected $b;
public function __construct(B $b) {
$this->b = $b;
}
}
class B {
public $a;
protected function setA(A $a) {
$this->a = $a;
}This sort of dependency often means that you don’t really have two independent classes. Chances high, that these classes share one common responsibility, so maybe they both should be one class, rather than two separate.
Injecting Dependencies For Other Objects
An object receives a dependency that it doesn’t actually use, instead it simply passes this dependency to another object.
<?php
class DeliveryController
{
protected $http;
public function __construct(HttpClient $http)
{
$this->http = $http;
}
public function sendOrder($orderId)
{
$order = Order::find($orderId);
(new Delivery($this->http))->sendOrder($order);
}
}DeliveryController depends on HttpClient and doesn’t actually use it. The only reason to depend on HttpClient is to pass it later build an instance of Delivery class. The problem here is that we are coupled to Delivery class. It is hardcoded here with the new keyword. But even if we are not going to use another class here, we are still coupled to Delivery constructor. If its signature changes, the code here will be broken. We perfectly know this scenario: “I fixed it here, but now it is broken there”. To fix this issue, we can inject an already constructed instance of the Delivery class.
<?php
class DeliveryController
{
protected $delivery;
public function __construct(Delivery $delivery)
{
$this->delivery = $delivery;
}
public function sendOrder($orderId)
{
$order = Order::find($orderId);
$this->delivery->sendOrder($order);
}
}In other cases when you need to create a new object and pass some dependencies for it, factories can be a good solution. Simply inject a factory, that already has all the required dependencies. And then use it to build an object. Next time, when the process of instantiation changes, you should change only one line of code in the factory, instead of using find in project in your IDE.
Maybe Your Don’t Need It
Always think first before making any serious design decisions. Don’t follow any rules blindly. When all these gurus on Twitter post articles about DI and discuss it’s advantages, but you have some hardcoded dependencies in your code, that doesn’t automatically mean that you are not a professional developer. As any other technique, it has pros and cons. Previously we have discussed dependency injection only in a positive light. However, you can add all these layers of indirection for dependency injection to solve the problem that doesn’t really exist. If your class uses a dependency, but that concrete implementation is the only one available in your system, then it will be more painful to create a loosely coupled solution, rather than just use new keyword and forget about it.
Maybe you have Api class and it uses Guzzle client to send HTTP requests. It is very simle, it only knows the API endpoints and how to create requests.
<?php
namespace App\Api;
use GuzzleHttp\Client;
class Api
{
protected $client;
public function __construct()
{
$this->client = new Client();
}
public function getStatus($orderId)
{
$response = $this->client->get('https://example-api/status/' . $orderId);
return $this->parseResponse($response);
}
protected function parseResponse($response)
{
return json_decode($response, true);
}
}We can test Client class, but there is no solid reason to test Api class since it is only a wrapper over the Client. Tests for Api class, in reality, will only check the code for typos: expect this, and return that … expect this, and return that. So, we can simply hardcode the client in the constructor and use it.
But let’s see what happens if we follow good practices and use dependency injection in this case. At first, we need an interface HttpClient. Then we need to wrap a Guzzle client instance into a wrapper (adapter pattern), which implements this interface. Then we need a factory or dependency container, which will create this wrapper for us. And at last we can pass it to the constructor of Api class.
<?php
interface HttpClient {
// ...
}
class GuzzleAdapter implements HttpClient{
// ...
}
class Api
{
protected $client;
public function __construct(HttpClient $client)
{
$this->client = $client;
}
// ...
}And now ask yourself one question: what is the probability that you will use another HTTP client implementation, instead of the Guzzle client? In six months you will change it to Curl? I don’t think so. Is it worth it? You have created all these classes and complexity to replace only one line of code, which is unlikely to be changed.
Want to know more?
