In a study session at work last week we were having a discussion about how with dependency injection you can end up with loads of anaemic interfaces. Arguably these interfaces provide little value. However in C# at least there’s an alternative (thanks Josh and I think also Joe Campbell for this idea) – instead of taking an interface for a dependency in the client’s constructor, use a delegate instead.
So, instead of this “poor man’s” dependency injection example from my last post:
public interface IUserPaymentRepository
{
void MakeTransaction(Price amount);
}
public class TrackPaymentActivity
{
private UserPaymentRepository _userPaymentRepository;
public TrackPaymentActivity():this(new UserPaymentRepository())
{
}
public TrackPaymentActivity(IUserPaymentRepository userPaymentRepository)
{
this._userPaymentRepository = userPaymentRepository;
}
public AttemptToPayForTrack()
{
......
_userPaymentRepository.MakeTransaction(trackPrice);
......
}
}
you can do this:
public class TrackPaymentActivity
{
private Action _makeTransaction;
public TrackPaymentActivity(Action makeTransaction)
{
this._makeTransaction = _makeTransaction;
}
public AttemptToPayForTrack()
{
......
_makeTransaction(trackPrice);
......
}
}
So how do you test this? Mocking frameworks don’t (and probably couldn’t) support delegates so you’ll need to create an interface which has a method with the signature of the delegate, but only for testing purposes:
internal interface ITestPaymentTransaction
{
void MakeTransaction(Price amount)
}
[Test]
public void Should_Take_Correct_Payment_Amount_For_Track_From_User()
{
IUserPaymentRepository mockedTransaction =
MockRepository.GenererateMock();
new TrackPaymentActivity(mockedTransaction.MakeTransaction)
.AttemptToPayForTrack();
mockedTransaction.AssertWasCalled(transaction => transaction.MakeTransaction(expectedAmount));
}
In most situations I think this is preferable. You’re still having to create an interface, but it’s not creating noise in your production code. It also means you can’t use an IoC container, but as I said in my last post, in many situations you probably don’t need to anyway.
Fact : Using IoCs can indeed lead to an explosion of simple, single method being the more extreme case, interfaces, leading to very little overall cohesion within the code base. I see that happening every day at work.
Fact : Sometimes we don’t have the time to design interfaces (or other types for that matter) that have a rich API. I guess we only care with the problem in hand at the moment and using IoCs and using TDD/BDD can lead to the situation you’ve described. Would a TDD/BDD practicioner think that once the result has been achieved that maybe there is code reuse somewhere or enough similarility that it’s worth introducing new abstractions to bring it all together ?
Fact : A lot of systems are built upon rather ceremonious bits. We have our UI, we have maybe our Queries or Commands, we have our Service Layers, we have our Domain and we may have some more, too. Sometimes, none of these layers ever modify data that might come from a layer down that need to go up the layer hierarchy. A lot of systems are built like that and they, I think, end up having a lot of small interfaces that will be implemented by stateless types.
Now, I’d say your solution to substitute interfaces with delegates (Func, Action and all their variants apply) is horrible. A delegate is only defined by its type and the very nature of what .Net 3+ offers now we can create a delegate using generics which means we don’t ever have a delegate type we could add metadata (like documentation), so there is nothing in hell somebody would know that such Action is a counter and the other Action returns the the current hour.
I know a guy at work tried to push for that but we resisted, I think using a delegate within a set of closely related classes is perfectly valid solution though.
What you’ve raised is I don’t think a direct side-effect of using IoCs, just a side-effect of not refactoring code to promote code readibility/reuse in a system.
Daniel
Hi Daniel,
This feels like a bit of a straw man argument to me. This approach is all about balancing the signal to noise ratio in your code. Use it when it removes noise but don’t do if it adds to it.
Rob
I don’t think it’s a straw man argument.
Say you have “int ICounter.Next()” and “int IClock.Minutes {get;}”
Both of these interfaces could be replaced with Func if you wanted to.
If in my code I have a dependency on Func, I’d have to check where it’s declared to know what it means, .Net allows you to create lambda expressions therefore inlining implementation of say Func.
So to me, swapping from single method interfaces to delegates might be removing noise but you’re adding confusion to your codebase and not only that you no longer do static code analysis if that’s what you like doing.
I am personally keener on creating interfaces that provide a set of cohesive operations, unlike the tendency of creating single usage interfaces so I can see where you’re coming from.
Daniel
I’m getting C++ function pointer flashbacks. Seriously, don’t go there!
The goal is substitutability and the safe OO mechanism for that is polymorphism, not delegates! I think it’s true that applying IoC ubiquitously as a design philosophy (which it isn’t) rather than as a design patterm designed to solve a specific kind of problem is what leads to having too many interfaces. That these interfaces are “anaemic” isn’t a problem. We should present small, client-specific interfaces (See “Interface Segregation Principle”). That there are too many interfaces? Well, code riddled with delegates would have me reaching for the gin first, I think. Been there. Done that. Never again! 🙂
The only effective way to manage the dependencies in your code is to consider tham on a case-by-case basis. Declaring interfaces (or delegates) for every client-supplier relationship is not the way forward. It is OKAY to have clusters off closely-related concrete classes wich are packaged together talking directly to each other. It is OKAY to pass instances of concrete types into methods and constructors as long as this is a one-to-one relationship. If A depends on B and we’re not crossing component or system boundaries, then that’s probably fine. If A, B and C all depend on D then I think there’s a real case for having a substitutible abstraction for D which they can bind to. I call it the “Reused Abstractions” design principle. And, of course, if there are specialisations of D then it’s a no-brainer. But if there’s just D and an interface ID and only A uses it, then something’s amiss in your design. That’s where this explosion of small interfaces often comes from – people routinely decalring interfaces that participate in only one relationship and are implemented only once. Mocking frameworks and IoC containers have a lot to answer for in that respect.