HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Unit test if an event has been raised or not

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
raisedbeenhastestnotunitevent

Problem

Given the following event dispatcher code

```
public interface IApplicationEvent { }

public delegate void ApplicationEventHandlerDelegate(TEvent @event) where TEvent : IApplicationEvent;

public class ApplicationEventDispatcher
{
private bool _disposed;
private Dictionary _applicationEventHandlers;

public ApplicationEventDispatcher()
{
_applicationEventHandlers = new Dictionary();
}

~ApplicationEventDispatcher()
{
Dispose(false);
}

public void Dispose()
{
Dispose(true);
GC.SuppressFinalize(this);
}

private void Dispose(bool disposing)
{
if (_disposed) return;

if (disposing)
{
// free other managed objects that implement IDisposable only
RemoveAllListeners();
}

// release any unmanaged objects
// set the object references to null
_applicationEventHandlers = null;

_disposed = true;
}

public void AddListener(ApplicationEventHandlerDelegate handler)
where TEvent : IApplicationEvent
{
Delegate @delegate;
if (_applicationEventHandlers.TryGetValue(typeof(TEvent), out @delegate))
{
_applicationEventHandlers[typeof(TEvent)] = Delegate.Combine(@delegate, handler);
}
else
{
_applicationEventHandlers[typeof(TEvent)] = handler;
}
}

public void RemoveListener(ApplicationEventHandlerDelegate handler)
where TEvent : IApplicationEvent
{
Delegate @delegate;
if (_applicationEventHandlers.TryGetValue(typeof(TEvent), out @delegate))
{
Delegate currentDel = Delegate.Remove(@delegate, handler);

if (currentDel == null)
{
_applicationEventHandlers.Remove(typeof(TEvent));
}
else
{
_applicationEventHandlers[typeof(TEvent)] = currentDel;
}
}
}

Solution

Test naming

Given/when/then and arrange/act/assert are redundant and both are mostly noise. One would put comments like // ARRANGE etc in a test hundreds of lines long, usually legacy; as a reading help, before refactoring it.

Also method names are misleading. Most obvious one is whereas the method name says WhenAHandlerIsAttached the relevant snippet says:

// ACT
    dispatcher.Dispatch(new SimpleEvent1());
    dispatcher.Dispose();


Instead the method name should say when_an_event_is_dispatched.

Similarly GivenAnEventToDispatch is also misleading:

// ARRANGE
    ... @delegate = ...;
    var dispatcher = ...
    dispatcher.AddListener(@delegate);


Instead the method name should say given_an_event_dispatcher_and_an_event_handler_attached_to_it

I like CallsHandler and DoesNotCallHandler better, as you'll see further down; but in the name of consistency, given you have used given/when in the first part of the name, assertion part of the name should say then_the_handler_is_called then_the_handler_is_not_called.

My suggestion is below, I omitted some of the noise, but note the noisy parts are separated and pushed further down:

public class A_dispatcher {

    public void calls_a_handler_after_it_is_attached()
    {
        using(var dispatcher = aDispatcher())
        {
            dispatcher.AddListener(recordCall);

            dispatcher.Dispatch(anEvent());
        }

        Assert.IsTrue(callRecorder.IsCalled());
    }

    public void does_not_call_a_handler_after_it_is_detached()
    {
        using(var dispatcher = aDispatcher())
        {
            dispatcher.AddListener(recordCall);
            dispatcher.RemoveListener(recordCall);

            dispatcher.Dispatch(anEvent());
        }

        Assert.IsFalse(callRecorder.IsCalled());
    }

    private aDispatcher() {
        return new ApplicationEventDispatcher();
    }

    class CallRecorder
    {
        private bool called = false;

        public bool IsCalled() {return called;}

        public void Call()
        {
            this.Called = true;
        }
    }

    ... callRecorder = new CallRecorder();
    ... recordCall = callRecorder.Call;

}


Basic idea is you see this, if you look at he public declarations, e.g. in the class outline panels:

A_dispatcher
    calls_a_handler_after_it_is_attached
    does_not_call_a_handler_after_it_is_detached


Other points if your test case says AHandler your local variable name should not be @delegate.

Is the application event dispatch disposable or not? If you are implementing a Dispose method make the class implement IDisposable, or rename the method, if the class does not follow IDisposable contract.

After you do this you can eliminate the dispathcer.Dispose noise by using using. Or test set-up and clean-up methods.

Code Snippets

// ACT
    dispatcher.Dispatch(new SimpleEvent1());
    dispatcher.Dispose();
// ARRANGE
    ... @delegate = ...;
    var dispatcher = ...
    dispatcher.AddListener(@delegate);
public class A_dispatcher {

    public void calls_a_handler_after_it_is_attached()
    {
        using(var dispatcher = aDispatcher())
        {
            dispatcher.AddListener(recordCall);

            dispatcher.Dispatch(anEvent());
        }

        Assert.IsTrue(callRecorder.IsCalled());
    }

    public void does_not_call_a_handler_after_it_is_detached()
    {
        using(var dispatcher = aDispatcher())
        {
            dispatcher.AddListener(recordCall);
            dispatcher.RemoveListener(recordCall);

            dispatcher.Dispatch(anEvent());
        }

        Assert.IsFalse(callRecorder.IsCalled());
    }

    private aDispatcher() {
        return new ApplicationEventDispatcher();
    }

    class CallRecorder
    {
        private bool called = false;

        public bool IsCalled() {return called;}

        public void Call()
        {
            this.Called = true;
        }
    }

    ... callRecorder = new CallRecorder();
    ... recordCall = callRecorder.Call;

}
A_dispatcher
    calls_a_handler_after_it_is_attached
    does_not_call_a_handler_after_it_is_detached

Context

StackExchange Code Review Q#119689, answer score: 2

Revisions (0)

No revisions yet.