This morning I read an article this morning by Karl Seguin on allowing clients to replace system calls with delegates (function pointers) for testing purposes — making the untestable testable.
It is a pattern I have used myself recently, but under a different name with a more formalized syntax. Imagine, for example, you have created an interface to decouple e-mail sending dependencies:
|
1 2 3 4 |
public interface IMailSender
{
void Send(MailMessage message);
}
|
This allows me to swap out my IMailSender (via Dependency Injection) with a fake implementation for testing purposes – e.g. FakeMailSender or a mock.
Otherwise, production code uses SmtpMailSender, a concrete class I wrote that implements IMailSender via a call to System.Net.Mail.SmtpClient.Send():
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 |
public class SmtpMailSender : IMailSender
{
public event EventHandler<MailSentEventArgs> MailSent;
public void Send(MailMessage message)
{
if (message == null)
throw new ArgumentNullException("message");
new SmtpClient().Send(message);
MailSent(new MailSentEventArgs(message));
}
}
|
Note this class is a little bit special — it also has an event MailSent that gets raised every time a message is sent. This lets me attach stuff like global e-mail logging very easily, but how can we write a unit test that asserts this event gets raised at the correct time? Because of the dependency on SmtpClient.Send(), my SmtpMailClient class is now facing the exact same problem IEmailSender was designed to solve. For this issue you can check out this article as well.
Injecting an Action in the constructor
System.Net.Mail.SmtpClient.Send() isn’t virtual, so we can’t mock it, and I don’t really want another interface just for this situation. One solution Karl suggests is injecting an Action that does the actual sending:
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 |
public class SmtpMailSender : IMailSender
{
public event EventHandler<MailSentEventArgs> MailSent;
readonly Action<MailMessage> send;
public SmtpMailSender(Action<MailMessage> send)
{
this.send = send;
}
public void Send(MailMessage message)
{
if (message == null)
throw new ArgumentNullException("message");
send(message);
MailSent(new MailSentEventArgs(message));
}
}
|
This solves the dependency problem effectively and without creating new classes and interfaces. However, now users of the SmtpMailSender class are forced to provide the send action in the constructor.
|
1 2 |
var sender = new SmtpMailSender(new SmtpClient().Send);
sender.Send(...);
|
If you have a good IoC container it can take care of this, but other users may not be so lucky. There are a few other things that I didn’t like as well:
- 99% of this class’s functionality lies in this single action. A class where 99% of it’s functionality is injected in a single parameter raises the question why it really needs to exist at all.
- The default implementation, SmtpClient.Send(), only needs to be overriden in a few test cases. Everyone else shouldn’t have to care about it.
- Putting random delegates in a constructor makes me feel uncomfortable. Unless it is an intention-revealing named delegate, I don’t think this is a good pattern to be promoting.
Using Supersede Instance Variable instead
In Working Effectively with Legacy Code, Michael Feathers discusses a pattern called Supercede Instance Variable. Although it is used for technical reasons (replacing global dependencies in non-virtual C++ classes, where methods cannot be overridden via subclassing), I believe the pattern fits this usage example well.
Really, the only difference is that, here, is that unless a user performs the optional step of overriding the action via a special SupersedeSendActionmethod, a default is used:
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 |
public class SmtpMailSender : IMailSender
{
public event EventHandler<MailSentEventArgs> MailSent;
Action<MailMessage> send;
public Action<MailMessage> DefaultSend
{
get { return new SmtpClient().Send; }
}
public SmtpMailSender()
{
this.send = DefaultSend;
}
public void Send(MailMessage message)
{
if (message == null) throw new ArgumentNullException("message");
this.send(message);
MailSent(new MailSentEventArgs(message));
}
/// <summary>
/// Supersede the method that is used to send a MailMessage for testing
/// purposes (supersede static variable to break dependency on non-
/// mockable SmtpClient.Send() method).
/// </summary>
public void SupersedeSendAction(Action<MailMessage> newSend)
{
this.send = newSend;
}
}
|
It is only a small difference, but as Feathers states, using the supersede term indicates it is a rare deviation from default behaviour, and for special cases only:
One nice thing about using the word supersede as the method prefix is that it is kind of fancy and uncommon. If you ever get concerned about whether people are using the superceding methods in production code, you can do a quick search to make sure they aren’t.
Only supersede when you can’t inject
Remember though, that Supersede Instance Variable should only be used for very special cases like individual system library calls in low-level wrapper classes. Dependency Injection and intention-revealing interfaces is still a much better pattern, and should still be your primary tool in all other situations.
