February 7, 2013
If you have been reading some of the posts in my blog, you will know that sometimes I present a problem and a solution in an article which takes you through the thought process from making the first straightforward solution through refining that solution into a hopefully better code design. This is one of those posts.
Let’s talk a bit about separation of concerns. One scenario that I stumble upon fairly frequently when coding is the need for some code that should perform a task at a time based interval. This is nothing strange, and it is not very hard to achieve. But it still does present some challenges.
In order to have a simple example, let’s say that we have a TimeTeller class. It will, at a certain interval, tell us what time it is:
var timeTeller = new TimeTeller(
timeString => Console.WriteLine("The time is now {0}", timeString))
{
Interval = TimeSpan.FromSeconds(1)
};
timeTeller.Start();
It takes a callback method which will be called at the given interval, providing a string with the current time. Nice and simple. But it has two distinct drawbacks:
TimeTeller class now has two concerns: telling time, and maintaining an interval mechanismFirst, let’s look at a first implementation of the TimeTeller class, as it is being used above:
public class TimeTeller
{
private readonly Action timeStringCallback;
private TimeSpan interval;
private Timer timer;
public TimeTeller(Action timeStringCallback)
{
if (timeStringCallback == null) throw new ArgumentNullException("timeStringCallback");
this.timeStringCallback = timeStringCallback;
}
public TimeSpan Interval
{
get
{
return interval;
}
set
{
interval = value;
if (timer != null)
{
Start();
}
}
}
public void Start()
{
if (timer == null)
{
timer = new Timer(state => TellTime());
}
timer.Change(Interval, Interval);
}
public void Stop()
{
if (timer != null)
{
timer.Dispose();
timer = null;
}
}
private void TellTime()
{
timeStringCallback(DateTime.Now.ToString("HH:mm:ss"));
}
}
As you can see, most of the code in the class is actually not about telling time, but rather about maintaining the interval using a Timer. In this code, telling time is more of a side effect than anything else. Also, if I would want to test this, it would involve some sort of waiting mechanism:
using (AutoResetEvent waitHandle = new AutoResetEvent(false))
{
var timeTeller = new TimeTeller(s => { waitHandle.Set(); });
timeTeller.Start();
Assert.IsTrue(waitHandle.WaitOne(TimeSpan.FromSeconds(5)), "Callback method was not invoked.");
}
This is not overly complicated, but as a test it has one flaw: it can fail for reasons that are not related to our code. If the build server happens to be under great pressure (or just very slow), perhaps the code is not executed as fast as we expect so the wait handle times out. Our code may be perfectly correct, but the test still fails. I am strongly of the opinion that tests should be designed in a way so they should fail only if there is something wrong with the code of the application, not because of anomalies in the surrounding environment.
Now my code base has two problems. The test that is not optimal, and the fact that TimeTeller violates the Single Responsibility Principle.
In the Solid Responsibility Principle, a responsibility is sometimes described as a reason to change. The TimeTeller class in its current design may change for more than one reason: if we want to change how it is triggered, and if we want to change how it tells time.
So, first I want to remove the trigging mechanism from the TimeTeller class. To do this, I design an ITrigger interface:
public interface ITrigger
{
void RegisterAction(Action action);
void UnregisterAction(Action action);
}
It’s extremely simple; I can register an Action that will be invoked by the trigger, and I can unregister the Action. Notice how this interface says nothing at all about how the code will be trigged. From this point of view, that is of no importance. That is somebody else’s concern. Now I can refactor the TimeTeller class to use an external trigger instead of maintaining its own trigging mechanism:
public class TimeTeller
{
private readonly Action timeStringCallback;
private readonly ITrigger trigger;
public TimeTeller(Action timeStringCallback, ITrigger trigger)
{
if (timeStringCallback == null) throw new ArgumentNullException("timeStringCallback");
if (trigger == null) throw new ArgumentNullException("trigger");
this.timeStringCallback = timeStringCallback;
this.trigger = trigger;
}
public void Start()
{
trigger.RegisterAction(TellTime);
}
public void Stop()
{
trigger.UnregisterAction(TellTime);
}
private void TellTime()
{
timeStringCallback(DateTime.Now.ToString("HH:mm:ss"));
}
}
Notice how this class now has a much more focused design. There is a lot less code that is about how to do stuff. Let’s say it has a lower “how-to-what” ratio.
Great, but what about the trigger part? The code that I just ripped out of the TimeTeller class needs to go somewhere, right? Right. I need to implement some sort of interval based trigger. Already at this point I realize that I will want to implement a couple of different triggers. I want the interval trigger to use with my TimeTeller class. I will also want to make some sort of direct trigger that I can use to directly trig the functionality, which might be very useful in unit tests in order to “fake” the interval based trigging. So, I put the most basic stuff in a base class:
public abstract class TriggerBase : ITrigger
{
private Action action = () => { };
protected TriggerBase()
{
Enabled = true;
}
public bool Enabled { get; set; }
public void RegisterAction(Action action)
{
this.action = (Action)Delegate.Combine(this.action, action);
}
public void UnregisterAction(Action action)
{
this.action = (Action)Delegate.Remove(this.action, action);
}
protected void TrigInternal()
{
if (Enabled)
{
action.Invoke();
}
}
}
This takes care of the bare necessities: it allows me to register and unregister Action callbacks with the trigger, and have them invoked. Note that by using Delegate.Combine and Delegate.Remove, I can register multiple actions with the same trigger, which might be handy. Now, let’s extend the base class to make the interval trigger:
public class IntervalTrigger : TriggerBase
{
private TimeSpan interval;
private Timer timer;
public IntervalTrigger()
{
StartInterval();
}
public TimeSpan Interval
{
get
{
return interval;
}
set
{
interval = value;
StartInterval();
}
}
private void StartInterval()
{
if (timer == null)
{
timer = new Timer(state => TrigInternal());
}
timer.Change(Interval, Interval);
}
private void StopInterval()
{
if (timer != null)
{
timer.Dispose();
}
}
}
You can see how the IntervalTrigger implementation is very much like the code we removed from TimeTeller. Now when we use the TimeTeller class, we simply provide an IntervalTrigger that will take care of calling into the TimeTeller at the given interval:
var timeTeller = new TimeTeller(
timeString => Console.WriteLine("The time is now {0}", timeString),
new IntervalTrigger { Interval = TimeSpan.FromSeconds(1) });
timeTeller.Start();
Now that we have separated the concerns how to call code at a given interval and how to tell time, we also get a much nicer setup for testing. As I mentioned previously, I was planning to implement some sort of direct trigger that can be used for instance in tests:
public class DirectTrigger : TriggerBase
{
public void Trig()
{
TrigInternal();
}
}
By passing a DirectTrigger to TimeTeller, I can now easily control exactly when it is being called into, and it is done in a synchronous manner. The asynchronous test above can be rewritten into a synchronous one instead:
bool callbackWasInvoked = false;
var trigger = new DirectTrigger();
var timeTeller = new TimeTeller(s => { callbackWasInvoked = true; }, trigger);
timeTeller.Start();
trigger.Trig();
Assert.IsTrue(callbackWasInvoked, "Callback method was not invoked.");
This test is not time sensitive anymore, but will be very tolerant for slow build environments and such.
One nice thing about having moved the whole trigging concern out of the TimeTeller is that we can now allow the code to be trigged in new ways, without the TimeTeller even knowing about it. I used this fact when rewriting the test for instance. But what if I suddenly got the requirement that the TimeTeller should be able to tell time at a given interval, but there should also be a possibility to invoke it directly, at will. As it happens, this is now very easy to achieve. I just make a new ITrigger implementation:
public class CompositeTrigger : TriggerBase
{
public CompositeTrigger(params ITrigger[] triggers)
{
foreach (ITrigger trigger in triggers)
{
Add(trigger);
}
}
public void Add(ITrigger trigger)
{
trigger.RegisterAction(TrigInternal);
}
public void Remove(ITrigger trigger)
{
trigger.UnregisterAction(TrigInternal);
}
}
This gives me the option to combine any triggers I want, and the TimeTeller is still unaware:
DirectTrigger directTrigger = new DirectTrigger();
IntervalTrigger intervalTrigger = new IntervalTrigger
{
Interval = TimeSpan.FromSeconds(5)
};
CompositeTrigger trigger = new CompositeTrigger(directTrigger, intervalTrigger);
var timeTeller = new TimeTeller(s => { callbackWasInvoked = true; }, trigger);
This will cause TimeTeller will be invoked once every 5 seconds, and whenever any code calls directTrigger.Trig(). In the sample project that I have linked to in the end there is also an AsyncTrigger which is a flavor of the DirectTrigger that will invoke the target callbacks on a separate thread, so that the calling code is not blocked.
To sum it up, by separating concerns like this you will hopefully end up with a code design where both types and tests are more focused on their primary task. There will be more whatand less howin the code.