Exceptions During EventHandler Invocation and Allowing No Subscribers

Ever create a middle-tier library and have a ton of EventHandlers / EventArgs on your classes because you think it will be convenient in some use cases that just aren't needed up front?

In my experience, this always winds up as an annoyance on invocation as the end user of the library would need to wire up all those events even if they weren't being used, or risk a NullReferenceException saying "Object reference not set to an instance of an object".

This post shows a quick and easy pattern to use across any application or library to make consuming events completely optional. You can tell your classes "if no one is listening, don't invoke".

Lets start out with a simple console application and a few classes. We have a forest with some trees:

public class Forest
{
 public int TreeCount { get; private set; }
 public Forest(int numberOfTrees)
 {
  TreeCount = numberOfTrees;
 }

 public void RemoveATree()
 {
  TreeCount--;
 }

 public bool HasTrees { get { return TreeCount > 0; } }
}
And a Chopper that knows how to cut down trees in that forest.
public class TreeChopper
{
    private readonly Forest _forest;
    public TreeChopper(Forest aForest)
    {
        _forest = aForest;
    }

    public void ChopATreeDown()
    {
        if (!_forest.HasTrees)
        {
            throw new NoTreesLeftException("There are no trees in the forest to chop down.");
        }

        OnTreeChopping.Invoke(this, new TreeChoppingEventArgs("Starting to chop a tree."));
        OnTreeChopped.Invoke(this, new TreeChoppingEventArgs("Finished chopping."));
        OnTreeFalling.Invoke(this, new TreeChoppingEventArgs("Tree is starting to fall down."));
        _forest.RemoveATree();
        OnTreeFell.Invoke(this, new TreeChoppingEventArgs("Tree has fallen. There are "+_forest.TreeCount+" trees left in the forest."));
    }

    public EventHandler<TreeChoppingEventArgs> OnTreeChopping;
    public EventHandler<TreeChoppingEventArgs> OnTreeChopped;
    public EventHandler<TreeChoppingEventArgs> OnTreeFalling;
    public EventHandler<TreeChoppingEventArgs> OnTreeFell;

}

Note that there's 4 events for stages of chopping down that tree, and a simple message EventArgs:

public class TreeChoppingEventArgs : EventArgs
{
    public string Message { get; set; }
    public TreeChoppingEventArgs(string message)
    {
        Message = message;
    }
}

public class NoTreesLeftException : ArgumentOutOfRangeException
{
    public NoTreesLeftException(string message) : base(message){}
}

This is all well and good, and if we set up a runner, we can chop down all the trees in a little console app. The chopping:

public static void Run()
{
    //1. set up a forest.
    var forest = new Forest(numberOfTrees:5);

    //2. wire up all events.
    var chopper = new TreeChopper(forest);

    chopper.OnTreeChopping += OnTreeChoppingEvent;
    chopper.OnTreeChopped += OnTreeChoppingEvent;
    chopper.OnTreeFalling += OnTreeChoppingEvent;
    chopper.OnTreeFell += OnTreeChoppingEvent;


    //3. cut down all trees in the forest
    while (forest.HasTrees)
    {
        chopper.ChopATreeDown();
    }
}

private static void OnTreeChoppingEvent(object sender, TreeChoppingEventArgs e)
{
    Console.WriteLine(e.Message);
}

So this displays the following output:

Starting to chop a tree.
Finished chopping.
Tree is starting to fall down.
Tree has fallen. There are 4 trees left in the forest.
Starting to chop a tree.
Finished chopping.
Tree is starting to fall down.
Tree has fallen. There are 3 trees left in the forest.
Starting to chop a tree.
Finished chopping.
Tree is starting to fall down.
Tree has fallen. There are 2 trees left in the forest.
Starting to chop a tree.
Finished chopping.
Tree is starting to fall down.
Tree has fallen. There are 1 trees left in the forest.
Starting to chop a tree.
Finished chopping.
Tree is starting to fall down.
Tree has fallen. There are 0 trees left in the forest.


But that's pretty verbose. Lets say from the outside perspective, we don't care when the tree chopping started... or even finished. The only thing we care about is when that tree fell, so we can drag it away and use the wood. How do we invoke all of the methods, in case they are used, but get protected in case they are not listened to?

The answer lies in how events work. An event handler is simply a way of keeping track of how to call the delegates -- the places that the event truly gets 'handled'.

Let's take a look at the event handler's definition (.NET 4.0 EventHandler<TEventArgs> where TEventArgs:EventArgs )

This is essentially a subscription list. If you've wired up a handler for the event, it gets added to the list (really an Array). If not, the list is empty.

So to make an event that ensures it's only fired when there's someone listening the assumption is that we simply have to do the following:

Before:

OnTreeChopping.Invoke(this, new TreeChoppingEventArgs("Starting to chop a tree."));

After:

if (OnTreeChopping.GetInvocationList() != null)
{
OnTreeChopping.Invoke(this, new TreeChoppingEventArgs("Starting to chop a tree."));
}

Yet in practice, you'll find that this still throws the NullReferenceException. Why? Well, the OnTreeChopping object (the EventHandler itself) turns out to be null. So the first reaction is just to do the following. Check to see if the EventHandler is null, then invoke it if it's not.

Before:

OnTreeChopping.Invoke(this, new TreeChoppingEventArgs("Starting to chop a tree."));

After:

if (OnTreeChopping != null)
{
OnTreeChopping.Invoke(this, new TreeChoppingEventArgs("Starting to chop a tree."));
}

So now, if we wrap all of our events in this little block, they'll be 'safe', right? Well, lets assume the answer to that is true for the moment. Writing the same thing over and over again would be smelly; we've just made it 4 lines of code (well, 2 + braces) every time we want to invoke an event. Enter a handy extension method:

public static class EventHandlerExtensions
{
    public static void TryInvoke<T>
        (this EventHandler<T> anEvent, object sender, T eventArgs)
        where T:EventArgs
    {
        if (anEvent != null)
        {
            anEvent.Invoke(sender, eventArgs);
        }
    }    
}

So in our calling code:

Before:

OnTreeChopping.Invoke(this, new TreeChoppingEventArgs("Starting to chop a tree."));

After:

OnTreeChopping.TryInvoke(this, new TreeChoppingEventArgs("Starting to chop a tree."));

And if we only care about the tree being done falling, we can comment out the wiring for the other events as so:

//chopper.OnTreeChopping += OnTreeChoppingEvent;
//chopper.OnTreeChopped += OnTreeChoppingEvent;
//chopper.OnTreeFalling += OnTreeChoppingEvent;
chopper.OnTreeFell += OnTreeChoppingEvent;

The invocation as so:

OnTreeChopping.TryInvoke(this, new TreeChoppingEventArgs("Starting to chop a tree."));
OnTreeChopped.TryInvoke(this, new TreeChoppingEventArgs("Finished chopping."));
OnTreeFalling.TryInvoke(this, new TreeChoppingEventArgs("Tree is starting to fall down."));
_forest.RemoveATree();
OnTreeFell.TryInvoke(this, new TreeChoppingEventArgs("Tree has fallen. There are " + _forest.TreeCount + " trees left in the forest."));

And our output looks like so:
Tree has fallen. There are 4 trees left in the forest.
Tree has fallen. There are 3 trees left in the forest.
Tree has fallen. There are 2 trees left in the forest.
Tree has fallen. There are 1 trees left in the forest.
Tree has fallen. There are 0 trees left in the forest.

 

It's all nice and easy; no messages about chopping, which I don't care about. If the event never gets wired, it doesn't get called. Great. But that begs the question of why to even provide GetInvocationList anyway. Is it useful to us at all? Well, it turns out that there are cases where a misbehaving (read: exception throwing) event handler can spoil invocation for the rest of the handlers.


Chris Tacke had a great writeup on this here (http://blog.opennetcf.com/ctacke/2010/05/27/WhyYouShouldUseEventHandlerGetInvocationList.aspx), and I'm going to apply this scenario to our example. Lets modify our code somewhat to create something bad happening. Lets say we have two event handlers. One transmits the message about the tree having fallen to an RSS feed, and another logs it to some sql database. The sql database server just went down, but we still want to publish the live events to external consumers depending on the data for estimating how much wood they could have available for their production line. First of all, our extension method is now going to swallow the Exceptions, and write them nicely to our console output:

public static void TryInvoke<T>(this EventHandler<T> anEvent, object sender, T eventArgs)
    where T:EventArgs
{
    if (anEvent != null)
    {
        try
        {
            anEvent.Invoke(sender, eventArgs);    
        }
        catch (Exception e)
        {
            Console.WriteLine(e.Message);
        }
    
        
    }
}
Second, we're going to create a 'good' event handler, and a 'bad' one:
private static void OnTreeChoppingEventLogToSql(object sender, TreeChoppingEventArgs e)
{
    Console.WriteLine(e.Message);
    throw new InvalidDataException("sql database does not exist");
    Console.WriteLine("Logged to sql.");
}

private static void OnTreeChoppingEventLogToRssFeed(object sender, TreeChoppingEventArgs e)
{
    Console.WriteLine(e.Message);
    Console.WriteLine("Logged to RSS feed.");
}

And when we wire them up:

chopper.OnTreeFell += OnTreeChoppingEventLogToSql;
chopper.OnTreeFell += OnTreeChoppingEventLogToRssFeed;

So what does the output look like?
Tree has fallen. There are 4 trees left in the forest.
sql database does not exist
Tree has fallen. There are 3 trees left in the forest.
sql database does not exist
Tree has fallen. There are 2 trees left in the forest.
sql database does not exist
Tree has fallen. There are 1 trees left in the forest.
sql database does not exist
Tree has fallen. There are 0 trees left in the forest.
sql database does not exist

Wait... what happened there? What about that critical RSS feed? Well, it turns out that the exception thrown blocked further invocation of other event handlers. Keep in mind that event handlers are invoked in the order that they are added to the invocation list. Also, an exception by default prevents further invocation. So if we want to change that behavior, we'll need to manually call each EventHandler<T>, rather than just let the MulticastDelegate handle it.

So our extension method changes to:

public static void TryInvoke<T>(this EventHandler<T> anEvent, object sender, T eventArgs)
    where T:EventArgs
{
    if (anEvent != null)
    {
        foreach (EventHandler<T> wiredHandler in anEvent.GetInvocationList())
        {
            try
            {
                wiredHandler(sender, eventArgs);
            }
            catch (Exception e)
            {
                Console.WriteLine(e.Message);
            }
        }
    }
}

Which makes our output work:
Tree has fallen. There are 4 trees left in the forest.
sql database does not exist
Tree has fallen. There are 4 trees left in the forest.
Logged to RSS feed.
Tree has fallen. There are 3 trees left in the forest.
sql database does not exist
Tree has fallen. There are 3 trees left in the forest.
Logged to RSS feed.
Tree has fallen. There are 2 trees left in the forest.
sql database does not exist
Tree has fallen. There are 2 trees left in the forest.
Logged to RSS feed.
Tree has fallen. There are 1 trees left in the forest.
sql database does not exist
Tree has fallen. There are 1 trees left in the forest.
Logged to RSS feed.
Tree has fallen. There are 0 trees left in the forest.
sql database does not exist
Tree has fallen. There are 0 trees left in the forest.
Logged to RSS feed.

Note that this won't really work as we want it to if we need to get our exceptions properly bubbled out after the TryInvoke call. To this end, we can introduce some Exception collecting,

including a wrapper for event errors:

public class EventInvocationException : Exception
{
    public List<Exception> Exceptions { get; set; }
    public string Message { get; set; }
    public EventInvocationException(string message, List<Exception> exceptions)
    {
        Message = message;
        Exceptions = exceptions;
    }
}

And then collect them as they are thrown:

public static void TryInvoke<T>(this EventHandler<T> anEvent, object sender, T eventArgs)
    where T:EventArgs
{
    var bubbledExceptions = new List<Exception>();
    if (anEvent != null)
    {
        foreach (EventHandler<T> wiredHandler in anEvent.GetInvocationList())
        {
            try
            {
                wiredHandler(sender, eventArgs);
            }
            catch (Exception e)
            {
                bubbledExceptions.Add(e);
            }
        }
    }

    if (bubbledExceptions.Count>0)
    {
        throw new EventInvocationException("At least one event handler threw an exception", bubbledExceptions);
    }
}

Now since this will be used in a high-level library, allow the invoker to specify allowed behavior, with a default of allowing all of the exceptions to bubble out:

public static class EventArgsExtensions
{
    public static void TryInvoke<T>(this EventHandler<T> anEvent, object sender, T eventArgs, bool allowMultipleExceptions=true)
        where T:EventArgs
    {
        if (allowMultipleExceptions)
        {
            anEvent.TryInvokeWithMultipleExceptions(sender, eventArgs);
        }
        else
        {
            anEvent.TryInvokeWithOnlyOneExceptionAllowed(sender, eventArgs);
        }
    }

    private static void TryInvokeWithOnlyOneExceptionAllowed<T>(this EventHandler<T> anEvent, object sender, T eventArgs)
        where T:EventArgs
    {
        if (anEvent!=null)
        {
            anEvent.Invoke(sender, eventArgs);
        }
    }

    private static void TryInvokeWithMultipleExceptions<T>(this EventHandler<T> anEvent, object sender, T eventArgs)
        where T:EventArgs
    {
        var bubbledExceptions = new List<Exception>();
        if (anEvent != null)
        {
            foreach (EventHandler<T> wiredHandler in anEvent.GetInvocationList())
            {
                try
                {
                    wiredHandler(sender, eventArgs);
                }
                catch (Exception e)
                {
                    bubbledExceptions.Add(e);
                }
            }
        }

        if (bubbledExceptions.Count>0)
        {
            throw new EventInvocationException("At least one event handler threw an exception", bubbledExceptions);
        }
    }
}

Now we can still call the TryInvoke on our event like before:

OnTreeFell.TryInvoke(this, new TreeChoppingEventArgs("Tree has fallen. There are " + _forest.TreeCount + " trees left in the forest."));

However if we want the first exception to really blow up, we can use the old-style plain invoke:

var allowMultipleExceptions=false;
OnTreeFell.TryInvoke(
this, 
new TreeChoppingEventArgs("Tree has fallen. There are " +         _forest.TreeCount + 
    " trees left in the forest."), 
allowMultipleExceptions);

And the best part is that all you have to do from this point on is to call TryInvoke instead of Invoke whenever you're publishing an event, and import the namespace of your extension method.

Posted on 8/24/2011 8:51:00 PM by Jason Nadal

Permalink | Comments |

Categories: design | codeQuality | software | refactoring | development

Tags:

Be the first to rate this post

  • Currently 0/5 Stars.
  • 1
  • 2
  • 3
  • 4
  • 5

Case Studies of Bad UI are Everywhere

I went out to dinner with friends last night, and got treated to a particularly bad example of UI design gone wild. Somehow this UI evolved over time.

 

So as you can see here, there’s a disaster of light switches. Some for fans and vents, others for lights. The best part is that there’s

  • another row of lights i couldn’t fit into this picture
  • actually another entire ‘panel’ in a different part of the restaurant! (smaller, though)

When looking at this from my perspective, the meaning is lost. The first thing that comes to mind is that there must be some better way to do this (an actual panel, perhaps?), second is that things must be rough when you just want to turn things on or off for one light.

We had to ask what the deal with the lights actually was – apparently it’s really rough to train people to close down at night, because some of the switches stay on, and some stay off.

The punch line of this is that they recently repainted. Apparently they’re half mislabeled now, as they didn’t spend too much effort worrying about putting the right cover plates back on the right switches.

How many times have you seen this in web UI design? This is a real world example of what a form with controls thrown everywhere would look like. It seems ridiculous when we see it on a wall, and it should seem just as ridiculous when we make a webform that just throws 23+ controls at you and says “have fun”.

So what could be done to actually fix this design? Here’s some potential solutions:

  • Group like-switches together. Perhaps “bar lights”, “main seating lights”, “main seating fans” etc.
  • Group switches by context together. Typically we turn this set of lights on at the same time – group those.
  • “Complex” switch – a master programmable control panel (this seems expensive, and a design cop-out to me)
  • Hide them. This is the interesting one. Right now, this is exposed to all of the customers, even those who don’t need to be made aware of the complexity. I wonder what brand psychologists would say about the subtle hints given off by having this in a dining room – it certainly wouldn’t make you feel like a tight ship was being run.If the complexity is a must (again, that should be a last resort), then only show it to those who must interact with it. Perhaps the closer touches the master panel, and the average wait staff only works with the “simple” panel.

One last point – if you see that much mess out front, imagine what’s going on behind the scenes. Behind every light switch should be a box that encases the electrical wiring behind the drywall – this is usually slightly larger than the switch itself. Imagine where all the boxes would fit in the above picture – there’s not much room for the wall at that point… and there must be just a mess of wiring in the wall.

Bad UI can expose your code smell to the client. This is the worst – it’s bad enough if your code is riddled with flaws and is an unmaintainable mess, but the client should not have to have that made visible to them!

Posted on 5/9/2009 9:29:00 AM by Jason Nadal

Permalink | Comments |

Categories: ui | design

Tags:

Be the first to rate this post

  • Currently 0/5 Stars.
  • 1
  • 2
  • 3
  • 4
  • 5