I was speaking with a colleague about the importance of reducing dependencies. I’ll reproduce here some work that we did to streamline some code and make it easier to test and maintain.
Consider this snippet as our starting point.
public Employee[] GetEmployees() {
var url = MyApplication.Settings.RemoteApi.Url;
var auth = new Auth(MyApplication.User);
var remoteApi = new RemoteApi(auth, url);
var rawData = remoteApi.FindEmployees();
var mapper = new EmployeeMapper();
var mappedResults= mapper.map(rawData);
return mappedResults;
}
Dependencies
MyApplication
MyApplication.Settings
MyApplication.Settings.RemoteApi
MyApplication.Settings.RemoteApi.Url
Auth
User
RemoteApi
FindEmployees
EmployeeMapper
That’s a lot objects or properties that will impact this code if they change for any reason. Further, when we attempt to write an automated test against this code we are dependent on a live existence of an API.
Law of Demeter
We can apply the Law of Demeter to reduce some of the dependencies in this code.
- Each unit should have only limited knowledge about other units: only units "closely" related to the current unit.
- Each unit should only talk to its friends; don’t talk to strangers.
- Only talk to your immediate friends.
In practice this usually means replacing code that walks a dependency hiearchy with a well-named function on the hierarchy root and call the function instead. This allows you to change the underlying hierarchy without impacting the code that depends on the function.
Consider the following modifications.
public Employee[] GetEmployees() {
var url = MyApplication.GetRemoteApiUrl();
var auth = MyApplication.GetAuth();
var remoteApi = new RemoteApi(auth, url);
var rawData = remoteApi.FindEmployees();
var mapper = new EmployeeMapper();
var mappedResults= mapper.map(rawData);
return mappedResults;
}
Dependencies
MyApplication
Auth
User
RemoteApi
FindEmployees
EmployeeMapper
I think we can do better.
public Employee[] GetEmployees() {
var remoteApi = MyApplication.GetRemoteApi();
var rawData = remoteApi.FindEmployees();
var mapper = new EmployeeMapper();
var mappedResults= mapper.map(rawData);
return mappedResults;
}
Dependencies
MyApplication
RemoteApi
FindEmployees
EmployeeMapper
This is starting to look good. We’ve reduced the number of dependencies in this function by half by implementing a simple design change.
Dependency Inversion
We still have the problem that we are dependent on a live instance of the API when we write automated tests for this function. We can address this by applying the Dependency Inversion Principle
- High-level modules should not depend on low-level modules. Both should depend on abstractions (e.g. interfaces).
- Abstractions should not depend on details. Details (concrete implementations) should depend on abstractions.
In practice this means that our GetEmployees
function should not depend on a concrete object. We can address this issue by passing the RemoteApi
to the constructor of the class as opposed to having the method get the RemoteApi
instance through MyApplication
.
public class EmployeeRepository {
private RemoteApi remoteApi;
public EmployeeRepository(RemoteApi remoteApi) {
this.remoteApi = remoteApi;
}
// snipped
public Employee[] GetEmployees() {
var rawData = this.remoteApi.FindEmployees();
var mapper = new EmployeeMapper();
var mappedResults= mapper.map(rawData);
return mappedResults;
}
Dependencies
RemoteApi
FindEmployees
EmployeeMapper
With this change we have reduced dependencies even further. However, we have not satisifed the Dependency Inversion Principle yet because we are still dependent on a concrete class: RemoteApi
. If we extract an interface from RemoteApi
and instead depend on that then the DIP is satisfied.
public interface IRemoteApi {
RawEmployeeData[] FindEmployees();
}
public class EmployeeRepository {
private IRemoteApi remoteApi;
public EmployeeRepository(IRemoteApi remoteApi) {
this.remoteApi = remoteApi;
}
// snipped
public Employee[] GetEmployees() {
var rawData = this.remoteApi.FindEmployees();
var mapper = new EmployeeMapper();
var mappedResults= mapper.map(rawData);
return mappedResults;
}
Dependencies
IRemoteApi
FindEmployees
EmployeeMapper
Now that we are dependent on an abstraction, we can provide an alternative implementation of that abstraction to our automated tests (a test double) that verify the behavior of GetEmployees
. This is a good place to be. Our code is loosely coupled, easily testable, and does not easily break when implementation details in other objects change.
I love this in theory, but isn’t the real dependency the RawEmployeeData array returned by the IRemoteApi.GetEmployees call? Why not just pass THAT to the constructor?
I’d suggest a further refactor:
public Employee[] GetEmployees() { var rawData = this.remoteApi.FindEmployees(); return employeeMapper.map(rawData); }
Because (1) even if you want to “new EmployeeMapper()”, you could keep that dependency with all of the other dependencies by instantiating the object in the constructor, and (2) there is no value in the “var mappedResults” communication if the method is “employeeMapper.map”, which obviously would return mapped results. (Something I don’t think you’d necessarily get with “mapper.map(rawData)”, which shows the danger of dropping the consistency of the naming of var mapper = new EmployeeMapper() over var employeeMapper = new EmployeeMapper()))
These are more than trival things, because look at what the function truly does: It gets data from an API, and it maps it. The original code was 7 lines, and the new code was 4… isn’t it magical when the shape of the code matches its function? It’s so close, let’s push it over the line! 🙂