Stormbase.net

When it comes to developing a new class, I don't tend to use TDD (Test Driven Development), I favour something I have named TAD - Test Aided Development. In other words, while I am for Unit Testing in general, designing something via writing tests sometimes feels too clunky and slow. I always write classes and methods with testing very much in mind, but I do not generally write the tests until later on in the process. This post covers roughly how I wrote the EventDistributor, and what points of note there are along the way.

The first phase in designing it, was the use case:

events.RegisterFor<PersonSavedEvent>(OnPersonSaved);
events.Publish(new PersonSavedEvent());
events.UnRegisterFor<PersonSavedEvent>(OnPersonSaved);

private void OnPersonSaved(PersonSavedEvent e)
{
    /* ... */
}

From this use case, we are able to tell that we will have 0 -> n events, and each event will have 0 -> n subscribers. This points to some kind of Dictionary based backing field:

public class EventDistributor
{
    private readonly Dictionary<Type, List<Action<Object>>> _events;

    public EventDistributor()
    {
        _events = new Dictionary<Type, List<Action<Object>>>();
    }

    public void RegisterFor<TEvent>(Action<TEvent> handler)
    {
    }

    public void UnRegisterFor<TEvent>(Action<TEvent> handler)
    {       
    }

    public void Publish<TEvent>(TEvent @event)
    {       
    }
}

For populating the dictionary, we need to add an entry for a TEvent if there is not already one (and create a blank list of handlers), and append our new handler:

public void RegisterFor<TEvent>(Action<TEvent> handler)
{
    var type = typeof(TEvent);
    List<Action<Object>> handlers;

    if (_events.TryGetValue(type, out handlers) == false)
    {
        handlers = new List<Action<Object>>();
        _events[type] = handlers;
    }

    handlers.Add(handler);
}

This gives rise to the first problem: the line handlers.Add(handler); gives us a nice error of: Error Argument '1': cannot convert from 'System.Action<TEvent>' to 'System.Action<Object>'. To fix this, we need to create a new Action<Object> and inside that, cast the parameter to TEvent.

handlers.Add(o => handler((TEvent) o));

This does however make the UnRegisterFor method a little more tricky, as doing handlers.Remove(o => handler((TEvent)o)); doesn't work because they refer to different objects. Thankfully, as the Action's GetHashCode() gives the same result for each instance, providing the content is the same. We can use this to check for equality:

public void UnRegisterFor<TEvent>(Action<TEvent> handler)
{
    var type = typeof(TEvent);
    List<Action<Object>> handlers;

    if (_events.TryGetValue(type, out handlers) == false)
    {
        return;
    }

    var hash = new Action<object>(o => handler((TEvent) o)).GetHashCode();
    handlers.RemoveAll(h => h.GetHashCode() == hash);
}

The Publish method is nice and straight forward; if the event isn't registered, throw an exception, and raise each subscriber's handler.

public void Publish<TEvent>(TEvent @event)
{
    var type = typeof(TEvent);
    List<Action<Object>> handlers;

    if (_events.TryGetValue(type, out handlers) == false)
    {
        throw new EventNotRegisteredException(type);
    }

    handlers.ForEach(h => h.Invoke(@event));
}

Now that we have a class roughly implemented, we create the first set of tests for it:

[Test]
public void When_publishing_an_event_without_a_handler()
{
    var distributor = new Distributor();
    Assert.DoesNotThrow(() => distributor.Publish(new PersonSavedEvent()));
}

[Test]
public void When_publishing_an_event_with_a_handler()
{
    var wasCalled = false;
    var distributor = new Distributor();

    distributor.RegisterFor<TestEvent>(e => wasCalled = true);
    distributor.Publish(new TestEvent());

    Assert.IsTrue(wasCalled, "The target was not invoked.");
}

[Test]
public void When_publishing_an_event_and_un_registering()
{
    var callCount = 0;
    var increment = new Action<TestEvent>(e => callCount++);
    var distributor = new Distributor();

    distributor.RegisterFor<TestEvent>(increment);
    distributor.Publish(new TestEvent());

    distributor.UnRegisterFor<TestEvent>(increment);
    distributor.Publish(new TestEvent());

    Assert.AreEqual(1, callCount);
}

Other than the publish method is currently a blocking operation, there is one major floor to this class: it contains a possible memory leak. If a class forgets to UnRegisterFor a handler, the EventDistributor will still have a reference stored, preventing the calling class from being garbage collected. We can demonstrate this with a simple unit test:

[Test]
public void When_the_handling_class_does_not_call_unregister()
{
    var count = 0;
    var increment = new Action(() => count++);
    var distributor = new Distributor();

    using(var l = new Listener(distributor, increment))
    {
        distributor.Publish(new TestEvent());
    }

    GC.Collect();
    GC.WaitForPendingFinalizers();
    GC.Collect();

    distributor.Publish(new TestEvent());

    Assert.AreEqual(1, count, "OnPersonSaved should have only been called 1 time, was actually {0}", count);
}

public class Listener : IDisposable
{
    private readonly Action _action;

    public Listener(Distributor events, Action action)
    {
        _action = action;
        events.RegisterFor<TestEvent>(OnTestEvent);
    }

    private void OnTestEvent(TestEvent e)
    {
        _action.Invoke();
    }

    public void Dispose()
    {
    }
}

While it would be simple to just say that it's the responsibility of the calling code to call UnRegisterFor, it would be better to handle that (likely) case ourselves. Good news is that .net has just the class needed for this built in: WeakReference. This class allows the target class to become disposed even while we still hold a reference to it. We can then act on the disposal, and remove our event registration.

Changing the Dispatcher to use this in its dictionary is fairly straight forward, and we even loose some of the casting needed to add items to the list:

public class Distributor
{
    private readonly Dictionary<Type, List<WeakReference>> _events;

    public Distributor()
    {
        _events = new Dictionary<Type, List<WeakReference>>();
    }

    public void RegisterFor<TEvent>(Action<TEvent> handler)
    {
        var type = typeof(TEvent);
        List<WeakReference> recipients;

        if (!_events.TryGetValue(type, out recipients))
        {
            recipients = new List<WeakReference>();
            _events[type] = recipients;
        }

        recipients.Add(new WeakReference(handler));
    }

    public void UnRegisterFor<TEvent>(Action<TEvent> handler)
    {
        var type = typeof(TEvent);
        List<WeakReference> recipients;

        if (_events.TryGetValue(type, out recipients))
        {
            recipients.RemoveAll(o => o.Target.GetHashCode() == handler.GetHashCode());
        }
    }

    public void Publish<TEvent>(TEvent @event)
    {
        var type = typeof(TEvent);
        List<WeakReference> recipients;

        if (!_events.TryGetValue(type, out recipients))
        {
            return;
        }

        recipients.RemoveAll(wr => wr.IsAlive == false);
        recipients.ForEach(wr => ((Action<TEvent>)wr.Target).Invoke(@event));
    }
}

The main points to note with this change is:

• We no longer need to create a new Action<Object> just to cast the handler in RegisterFor.
• UnRegisterFor no longer needs to create a new Action<Object> to get the hash code.
• Publish has an extra line to remove all handlers where the target has become disposed.

The next item to work on in this class is making the Publish method non-blocking, which can be done in a variety of ways.

The first option is to create a thread that will invoke all the handlers one after the other. This has the advantage of only one extra thread to deal with, but has the drawback of a single unresponsive handler will block all other handlers. Ignoring locking and cross-threading issues for the time being, it could be implemented like this:

public void PublishAsyncV1<TEvent>(TEvent @event)
{
    var type = typeof(TEvent);
    List<WeakReference> recipients;

    if (!_events.TryGetValue(type, out recipients))
    {
        return;
    }

    var task = new Task(() =>
    {
        recipients.RemoveAll(wr => wr.IsAlive == false);
        recipients.ForEach(wr => ((Action<TEvent>) wr.Target).Invoke(@event));
    });

    task.Start();
}

The second option is to have a separate thread/invocation for each handler. This has the advantage that each of the handlers can take as much time as needed, and will not block any other handlers from being raised, however if you have many handlers to be invoked, it could be slower to return than the first option. Again, ignoring locking and cross-threading issues, it could be implemented like so:

public void PublishAsyncV2<TEvent>(TEvent @event)
{
    var type = typeof(TEvent);
    List<WeakReference> recipients;

    if (!_events.TryGetValue(type, out recipients))
    {
        return;
    }

    recipients.RemoveAll(wr => wr.IsAlive == false);
    recipients.ForEach(wr =>
    {
        var handler = (Action<TEvent>)wr.Target;
        handler.BeginInvoke(@event, handler.EndInvoke, null);
    });
}

Personally, I go for the second method, as the number of handlers to be invoked is usually fairly small.

The next part to consider is what we conveniently ignored earlier - the cross-threading issues. The main issue we have is handlers being added or removed from the list while we are iterating over it.

Now I cannot remember where I read it, it was either from Jon Skeet, or from the Visual Basic .Net Threading Handbook, but the rough idea was "You should lock as smaller area of code as possible". This is to minimise the chance of a deadlock. Starting with the Publish methods, we only need to lock the parts that iterate over the list:

lock (Padlock)
{
    recipients.RemoveAll(wr => wr.IsAlive == false);
    recipients.ForEach(wr =>
    {
        var handler = (Action<TEvent>)wr.Target;
        handler.BeginInvoke(@event, handler.EndInvoke, null);
    });
}

The UnRegisterFor method is also very straight forward, as we again only need to worry about the iteration:

if (_events.TryGetValue(type, out recipients))
{
    lock (Padlock)
    {
        recipients.RemoveAll(o => o.Target.GetHashCode() == handler.GetHashCode());
    }
}

The RegisterFor method takes a little more locking than the other two, as this will handle the creation of the lists, as well as the addition to the list:

lock (Padlock)
{
    if (!_events.TryGetValue(type, out recipients))
    {
        recipients = new List<WeakReference>();
        _events[type] = recipients;
    }

    recipients.Add(new WeakReference(handler));
}

The full code listing and unit tests for this can be found here: EventDistributor Gist.

Table of Contents:

• Introduction
• Presenter to View Communication
• View to Presenter Communication
• Composite Views
• Presenter / Application communication
• ...

When working with MVP, it won't be long before you come across the need for multiple views on one form. There are several ways to achive this, and which you choose is really down to how you intend to (re)use your views.

The first method for dealing with the sub views is to expose them as a property of your main view, and set them up in the main view's presenter:

interface IMainView
{
    ISubView1 View1 { get; }
    ISubView2 View2 { get; }

    /* Other properties/methods etc for MainView */
}

class MainView : Form, IMainView
{
    public ISubView1 View1 { get { return this.subView1; } }
    public ISubView2 View2 { get { return this.subView2; } }
}

class MainPresenter
{
    private readonly IMainView _view;
    private readonly SubPresenter1 _pres1;
    private readonly SubPresenter2 _pres2;

    public MainPresenter(IMainView view)
    {
        _view = view;
        _pres1 = new SubPresenter1(view.View1);
        _pres2 = new SubPresenter2(view.View2);
    }

}

static class Program
{
    static void Main()
    {
        using (var view = new MainView())
        using (var presenter = new MainPresenter(view))
        {
            presenter.Display();
        }
    }
}

This method's advantage is simplicity, just create a new view and presenter, and call Display. The disadvantage is that the main presenter is tied to the sub presenters. A slight modification alleviates this:

interface IMainView
{
    ISubView1 View1 { get; }
    ISubView2 View2 { get; }

    /* Other properties/methods etc for MainView */
}

class MainView : Form, IMainView
{
    public ISubView1 View1 { get { return this.subView1; } }
    public ISubView2 View2 { get { return this.subView2; } }
}

class MainPresenter
{
    private readonly IMainView _view;
    private readonly SubPresenter1 _pres1;
    private readonly SubPresenter2 _pres2;

    public MainPresenter(IMainView view, SubPresenter1 pres1, SubPresenter2 pres2)
    {
        _view = view;
        _pres1 = pres1;
        _pres2 = pres2;
    }

}

static class Program
{
    static void Main()
    {
        using (var view = new MainView())
        using (var pres1 = new SubPresenter1(view.View1));
        using (var pres2 = new SubPresenter2(view.View2));
        using (var presenter = new MainPresenter(view, pres1, pres2))
        {
            presenter.Display();
        }
    }
}

The only change here is to pass our two sub presenters in to the main presenter as constructor parameters. Ultimately this seems to be the 'best' solution from a coupling point of view, however, if you are unlikely to change the sub presenters out for completely different sub presenters, then I would use the first method.

The final method for composing sub views is to push the responsibility to the actual main view, and make your main view pass any events and data to and from the sub view:

interface IMainView
{
    String FirstName { get; set; }
    String LastName { get; set; }

    String AddressLine1 { get; set; }
    String PostCode { get; set; }

    /* Other properties/methods etc for MainView */
}


class MainView : Form, IMainView
{
    private readonly SubPresenter1 _pres1;
    private readonly SubPresenter2 _pres2;

    void MainView()
    {
        InitializeComponent();
        _pres1 = new SubPresenter1(subView1);
        _pres2 = new SubPresenter2(subView2);

    }

    String FirstName 
    { 
        get { return subView1.FirstName; } 
        set {subView1.FirstName = value;} 
    }

    String LastName 
    { 
        get { return subView1.LastName; }
        set { subView1.LastName = value; }
    }

    String AddressLine1 
    { 
        get { return subView2.AddressLine1; }
        set { subView2.AddressLine1 = value; }
    }

    String PostCode 
    { 
        get { return subView2.PostCode; }
        set { subView2.PostCode = value; }
    }

}

The disadvantage to this is that if one of the subViews were to change in anyway, the MainView also has to change to reflect this.

Out of the three methods outlined, Method 2 is my personal preference, especially when not using a DI Container, and Method 2 when I am using one. The 3rd Method I find is too brittle for most usage, especially during earlier stages of development when the UI is more likely to be changing.

Table of Contents:

• Introduction
• Presenter to View Communication
• View to Presenter Communication
• Composite Views
• Presenter / Application communication
• ...

Communicating from the View to the Presenter is a reasonably straight forward affair. To signal something happening, we use an Event, but one with no parameters. We pass no parameters, as we are not going to be using them anyway, so what is the point is raising an event every time with OkayClicked(this, EventArgs.Empty)?

To get around this, we define a new event type, so that we can get rid of our redundant parameters:

public delegate void EventAction();

In the View we define our events:

public interface IEmployeesView
{
    event EventAction OkayClicked;
    event EventAction CancelClicked;
}

And in the Presenter we hook up the events:

public class EmployeesPresenter : IDisposable
{
    private readonly IEmployeeView _view;

    public EmployeesPresenter(IEmployeesView view)
    {
        _view = view;
        _view.OkayClicked += OnOkayClicked;
        _view.CancelClicked += OnCancelClicked;
    }

    private void OnOkayClicked() { /* ... */ }
    private void OnCancelClicked() { /* ... */ }

    public void Dispose()
    {
        _view.OkayClicked -= OnOkayClicked;
        _view.CancelClicked -= OnCancelClicked;
    }
}

Now I don't know about you, but I dislike having to wire and unwire an event like this - there is too much chance that I will either forget to unwire one of the events, or when copying and pasting the Add code (I know, I know) to the Remove section, I will forget to change a + to a -.

To this end, I created a class that will auto wire events to handlers based on a convention. It was based off of the Presenter base class written by Mark Nijhof in his Fohjin.DDD sample application, with a few improvements (namely the unwiring of events). To avoid the need to make your Presenters inherit from a base class (and to not violate SRP!), it is wrapped up into its own class, which can be used like so:

public class EmployeesPresenter : IDisposable
{
    private readonly IEmployeeView _view;
    private readonly EventAutoWirer<IEmployeeView> _autoWire;

    public EmployeesPresenter(IEmployeesView view)
    {
        _view = view;
        _autoWire = new EventAutoWire<IEmployeeView>(view, this);
        _autoWire.Wire();
    }

    private void OnOkayClicked() { /* ... */ }
    private void OnCancelClicked() { /* ... */ }

    public void Dispose()
    {
        _autoWire.Unwire();
    }
}

The wirer supports conventions to allow easier hook-up - by default it will only hook events defined in the View's interface to private methods prefixed with "On" in the presenter. This means that any events which are only defined in the concrete implementation of the View are not wired automatically - allowing extra view only functionality such as when an item is MouseOver'd etc.

Table of Contents:

• Introduction
• Presenter to View Communication
• View to Presenter Communication
• Composite Views
• Presenter / Application communication
• ...

Presenter to View Communication

There are two styles utilised for populating the View with data from the Presenter and Model that I have used. The only difference between them is how tightly coupled you mind your View being to the Model. For the example of this, we will have the following as our Model:

public class Person
{
    public int ID { get; private set; }
    public int Age { get; set; }
    public String FirstName { get; set; }
    public String LastName { get; set; }
    Public Genders Gender { get; set; }
}

Method 1: Using the Model

Now our View code:

public interface IEmployeesView
{
    void ClearList();
    void PopulateList(IEnumerable<Person> people);
}

And finally the Presenter:

public class IEmployeesPresenter
{
    public void Display()
    {
        _view.ClearList();
        _view.PopulateList(_model.AllEmployees);
    }
}

This method of population produces a link between the Model and the View; the Person object used as a parameter in PopulateList.

The advantage of this is that the concrete implementation of the IEmployeesView can decide on what to display in its list of people, picking from any or all of the properties on the Person.

There are two disadvantages of this method. The first is that there is nothing stopping the View from calling methods on the Person, which makes it easy for lazy code to slip in. The second is that if the model were to change from a List<Person> to a List<Dog> for instance, not only would the Model and the Presenter need to change, but so the View would too.

Method 2: Using Generic Types

The other method population relies on using Tuple<...>, KeyValuePair<,> and custom classes and structs:

Now our View code:

public interface IEmployeesView
{
    void ClearList();
    void PopulateList(IEnumerable<Tuple<int, String> names);
}

And finally the Presenter:

public class IEmployeesPresenter
{
    public void Display()
    {
        var names = _model.AllEmployees.Select(x => new Tuple<int, String>(x.ID, x.FirstName + " " + x.LastName));

        _view.ClearList();
        _view.PopulateList(names);
    }
}

The advantages of this method of population is that the Model is free to change without needing to update the View, and the View has no decisions to make on what to display. It also prevents the View from calling any extra methods on the Person, as it does not have a reference to it.

The down sides to this method, are that you loose strong typing, and discoverability - It is quite obvious what a Person is but what a Tuple<int, String> is less obvious.

Table of Contents

• Introduction
• Presenter to View Communication
• View to Presenter Communication
• Composite Views
• Presenter / Application communication
• ...

What is MVP?

I first came across MVP in Jeremy Miller's Build Your Own Cab series, and have been using and improving how I work with this style ever since. Model View Presenters tend to come in one of two forms: Passive View, and Supervising Controller. I am a fan of the Passive View variety, primarily for the testing aspect, but also as I find it provides me with the best level of separation.

The code ends up structured like this:

The View contains only code that enables control population and feedback. This means the odd For loop or similar to fill a grid from a property, or feedback object construction, along the lines of new SelectedRowData {ID = (int)row.Tag, Name = row[0].Value}. However, it would not contain any decision logic.

The Presenter contains code to transform Model data to something the View can display, and vice-verse. It also contains any view logic, such as if a CheckBox is selected, then a MenuItem becomes disabled.

The Model is the data to be displayed. This can either be an abstraction that encompasses several entities and business logic, or can be some entities themselves.