Dependency hell in the land of IoC
Using automated dependency injection has made the life of a developer a bit easier, or so it seems for a while. Maybe a bit too easy. In this post I’ll describe some of the mess this can lead to, and what you should do to avoid it, or fix it if it has already happened.
The examples and mentioned libraries are Java-based, but most languages have something similar.
What is dependency injection?
Dependency injection (DI) is at its core a way of passing dependencies to a class to it by some means.
It can be easily explained by some piece of code:
class NoDependencyInjection {
private OtherService os;
private DifferentService ds;
public NoDependencyInjection() {
os = new OtherService();
ds = new DifferentService();
}
}
class WithDependencyInjection {
private OtherService os;
private DifferentService ds;
public WithDependencyInjection(OtherService os, DifferentService ds) {
this.os = os;
this.ds = ds;
}
}
The main difference is that the class NoDependencyInjection is responsible for creating and managing its dependencies on its own, while the WithDependencyInjection-class does not care how the dependencies are created, its just passed them, and uses their exposed API.
One common problem with the NoDependencyInjection-class way of doing things is about testability. Given that OtherService talks to a database, how do you make it so that you can properly unittest NoDependencyInjection?
You’d have to create some mock database, and do a lot of setup for OtherService, which NoDependencyInjection really should know nothing about, or create some sort of factory, that you can for each test specify the sort of object to return, something like ServiceFactory.setService(myServiceStub). The problem with the factory method is that your tests cannot run at the same time, since you are most likely using the same factory instance to create the service.
In my opinion OtherService should be testet on its own, and NoDependencyInjection should just rely on OtherService to function the way it is supposed to.
Once a program reaches a certain size and complexity, the dependency setup starts to get a bit cumbersome. This led to the arrival of DI-frameworks. The Spring-project might contain the most used one, and provides the developer with the @Autowire-annotation. This tells the framework to inject a ready made instance of that dependency into to the class at runtime. This annotation has two basic usage patterns, field annotation and constructor annotation.
class Service {
@Autowired // Field annotation. Gets injected via reflection
private OtherService os;
@Autowired // Inject at construction
public Service(OtherService os) {}
}
The problem
One problem that often occurs are classes that have way more dependencies than any class should have. Automated dependency injections makes it, perhaps, a bit too easy to add a new dependency to a class, without properly thinking about the consequences. This is even easier on classes that have no tests attached to them, since you never see the cost of setting the class up to a functional state.
These classes tend to look something like this:
class MyService {
@Autowired
private OtherService os;
@Autowired
private OtherService2 os2;
@Autowired
private OtherService3 os3;
@Autowired
private OtherService4 os4;
@Autowired
private OtherService5 os5;
@Autowired
private OtherServiceN osN;
// .. methods are about five pages down
}
This is what I’d like to call legacy-by-design. It might work, but is nearly impossible to test (and if you manage it, it’s likely 500 lines of mock setup before you even get to test anything).
Why does this happen? For starters, It’s clearly not tested - the pain of testing this would have caused the developers to split the service long before that mess happened. Once a class gets, say 10 dependencies, the broken window effect starts to creep up on you.
Ugh, this is messy. Oh well, whats one more dependency? I’ll just add it.
Who cares about one more broken window in a house full of them?
So what can be done?
One small observation I’ve made is that if you use constructor based injection, it seems to deter people from adding more arguments more than adding a new field. My data set is quite small, so take this with a grain of salt, but the initial findings are promising.
So, if you use DI-frameworks, try to use constructor based injections. A nice bonus effect is that it is much easier to test, as you don’t have to bring up a Spring context in this instance, or something like Mockito’s @InjectMocks-magic. Still using Spring’s @Autowired annotation, this is easily done like this:
class MyService {
private final OtherService os;
@Autowired
public MyService(final OtherService os) {
this.os = os;
}
}
This only gets you so far though, and does not really fix the classes that are already broken. To fix these, you’ll just have to roll your sleeves up and get ready for the treachorous road of refactoring untested code. The book Working Effectively with Legacy Code has some great techniques for doing such work. But the gist of the processes is finding natural groups of functionality that can be moved to its own class. If no such groups are really present, which sometimes happens in complex integration work, you can consider moving some of the methods used from the dependencies to a new interface, with as few methods as possible, and create a facade class that proxies the calls to the dependencies. This allows you to stub the interface for tests more easily, and new patterns are likely to emerge.
Let’s see if we can create a decent example.
Given this class:
@Service
class BookingService {
@Autowired
private RoutingService routingService;
@Autowired
private AvailabilityService availabilityService;
@Autowired
private LocationRepository locationRepository;
@Autowired
private CustomerRepository customerRepository;
@Autowired
private StaffRepository staffRepository;
// .. Imagine a lot more dependencies here
public Booking requestBooking(
Long customerId,
WorkType workType,
DateTime dateOfWork
) {
Customer c = customerRepository.get(customerId);
Location l = locationRepository.get(c.homeAddress());
Set<Staff> possibleStaff = staffRepository.findForWorkType(workType);
Set<Staff> staffInRangeAndAvail = Sets.newHashSet();
for (Staff staffMember : possibleStaff) {
Long distance = routingService
.distanceBetween(l, staffMember.officeLocation());
if (distance < 10000 &&
availabilityService.isFree(staffMember, dateOfWork)) {
staffInRangeAndAvail.add(staffMember);
}
}
if (staffInRange.isEmpty()) {
return null;
}
return new Booking(staffInRange.get(0)); // Something like that
}
}
So, whats going on here. A quick glance shows that the requestBooking method uses five dependencies, which is quite a lot.
One possible route is to encapsulate the things that have to do with finding the staff members that have the right skills and are in range into a seperate class.
So, we can create a quick interface for the functionality
interface WorkorderStaffFinder {
Set<Staff> staffForWork(WorkType type, Location l, DateTime timeOfWork);
}
and a quick implementation
class DefaultWorkorderStaffFinder implements WorkorderStaffFinder {
private RoutingService routingService;
private AvailabilityService availabilityService;
private StaffRepository staffRepository;
@Autowired
public DefaultWorkorderStaffFinder(...) {} // assign dependencies
Set<Staff> staffForWork(
WorkType type,
Location l,
DateTime timeOfWork
) {
Set<Staff> possibleStaff = staffRepository.findForWorkType(workType);
Set<Staff> staffInRangeAndAvail = Sets.newHashSet();
for (Staff staffMember : possibleStaff) {
Long distance = routingService
.distanceBetween(l, staffMember.officeLocation());
if (distance < 10000 &&
availabilityService.isFree(staffMember, dateOfWork)) {
staffInRangeAndAvail.add(staffMember);
}
}
}
}
The original class now looks like this:
@Service
class BookingService {
@Autowired
private LocationRepository locationRepository;
@Autowired
private CustomerRepository customerRepository;
@Autowired
private WorkorderStaffFinder workorderStaffFinder;
// .. Imagine a lot more dependencies here
public Booking requestBooking(
Long customerId,
WorkType workType,
DateTime dateOfWork
) {
Customer c = customerRepository.get(customerId);
Location l = locationRepository.get(c.homeAddress());
Set<Staff> staff = workorderStaffFinder
.staffForWork(workType, l, dateOfWork);
if (staffInRange.isEmpty()) {
return null;
}
return new Booking(staffInRange.get(0)); // Something like that
}
}
We have removed two dependencies, and we can now more easily test the business
logic of requestBooking.
If you think about it, why should the requestBooking-method care how the
possible staff members are found? It should only care if some are found or not.
This is one aspect of the single responsibility principle that is a part of
SOLID (or some other buzzy-abbreviation).
Ideally we should end up with a BookingService-class that we can test
like this:
@Test
public requestBookingShouldReturnNullIfNoStaffIsAvailable() {
BookingService bs = new BookingService(
customerRepositoryMock,
locationRepositoryMock,
new EmptyResultWorkorderStaffFinder());
Booking b = BookingService.requestBooking(1L, WorkType.TECH, DateTime.now());
assertThat(b).isNull();
}
class EmptyResultWorkorderStaffFinder implements WorkorderStaffFinder {
Set<Staff> staffForWork(...) {
return new Set<Staff>();
}
}