Iteratively Decoupling Legacy Architecture

TL;DR: Move Service Locator calls up the object graph iteratively. ALSO: Poor Man's Dependency Injection breaks testing. Avoid it.

More and more, application architecture is migrating towards Dependency Injection to decouple components and ease testing. One of the challenges of applying this to existing applications is how to get the dependencies that an object requires to that object during construction. By design, constructor dependency injection means any object that instantiates another object also depends on the dependencies of the objects they create.

To illustrate this, let’s say I have created a ReportService, which creates an InvoiceQuery, which depends on DatabaseService to load query data, and then return a new Report object. As the ReportService is directly instantiating the InvoiceQuery object, it needs a reference to DatabaseService to pass to its constructor, and thus itself depends on DatabaseService.

This is one of the reasons factories and DI Containers are so useful - the ReportService shouldn’t need to depend on the InvoiceQuery’s own dependencies, so instead we can move this dependency management into the container for objects that are shared through the application or into factories for objects like our Query which are dynamically instantiated AFTER bootstrap

Refactoring an object to use dependency injection that is already used ubiquitously throughout the application and currently fetches its dependencies through service location becomes particularly challenging. One of the patterns often employed to try to solve this divide between dependency injection and service location is allowing objects to optionally receive their dependencies or obtain them via Service Location if they haven't been provided. We can think of this as Poor Man's Dependency Injection, as illustrated below:

Specifically, the constructor of the PoorMansQuery above includes the following line:
$this->db = $db ?: Services::getDbMysqlClient();

Untangling Implicit Coupling

Now when we write our Unit tests for PoorMansQuery we can optionally create a mock \Db_Mysql object and pass it in - successfully decoupling our code and using Dependency Injection, right? Unfortunately, whilst this does allow the PoorMansQuery object itself to be tested in isolation, by allowing the dependency to be optional any class which doesn't explicitly provide this dependency is now implicitly coupled to the service fetched from the container.

Instantiating the object is a tight coupling (we can't mock PoorMansQuery as it’s directly instantiated by the PoorManService), and as PoorManService doesn't provide the dependency to the Query's constructor, the service locator serves this dependency from the container - thus making it difficult to unit test PoorManService in isolation from the container. Not only this, the ternary operator within PoorMansQuery represents two behavioural pathways that our object supports. Unless we exercise the "missing dependency fetches from container" behaviour, we're not testing all the behaviours our object provides and thus haven't completely tested PoorMansQuery.

The fallback can be to bootstrap/setUp a dummy container containing mock dependencies and getting the Service Locator to instead receive this mock. Whilst this does provide a workaround, it’s cumbersome and fragile: we’re fiddling with global state, which risks breaking other tests if you forget to undo replacing the container, and you're still effectively coupled to a container.

The recommended way to avoid being coupled to a container is to construct all your dependencies during application dispatch via the DI Container. The challenge when refactoring existing legacy application code is this requires all objects that depend on the object you're creating/modifying to also be constructed via the DI Container, and so on and so forth all the way up your object graph, which can be a massive undertaking. However, whilst you may not have time to refactor the entire object graph, consider moving the Service Locator call up a level into where the objects are being constructed. As per the following refactored example:

This still doesn't decouple PoorManService from the container/Service Locator - and testing PoorManService will be equally challenging. Upon the next iteration, a developer working with the PoorManService can then "bubble" these Service Locator calls up to the next level and so on - thus decoupling PoorManService and allowing it to be unit tested in isolation. This also means we don't have to test the "missing dependency" behaviour provided by the PoorMansQuery as it no longer exists within the Query itself.

Iterative bubbling of Service Locator calls eventually brings service construction to the dispatch boundary, allowing us to move those dependencies into the container, greatly simplifying dependency management throughout the application. Eliminating Poor Man's DI is an excellent first step towards this goal.