On Thursday, I wrote about the importance of software patterns, specifically the event pattern in C#.

That very afternoon I was going through some code written by a co-worker that leveraged this pattern.

To keep things simple, let's imagine it was like this.

public event EventHandler NewMessage;

protected virtual void OnNewMessage(MessageEventArgs args)
{
  EventHandler handler = NewMessage;
  if(handler != null)
  {
    handler(this,args);
  }
}

protected virtual void OnNewMessage(string message)
{
  EventHandler handler = NewMessage;
  if(handler != null)
  {
    handler(this,new MessageEventArgs(message));
  }
}

Pretty straight forward, right? See the problem?

This gets to the core of why understanding a pattern deeply is important. The pattern of having a protected virtual OnFoo method for your Foo event is to ensure that a subclass can peek at the event before it's fired. The implementation above will require the subclass to not only override both methods, but to understand that she must override both methods to make sure all raised events are captured.

This is making the job of the subclass writer much harder and likely more error prone.

I'm not saying one should get rid of the helper method. It's fine. It just needs to be tweaked.

protected void OnNewMessage(string message)
{
  OnNewMessage(new MessageEventArgs(message));
}

First, get rid of the virtual flag. This should be callable from a subclass, but there should only be one method to override.

Second, call the 'real' virtual method from the helper. This ensures that anyone who overrides OnNewMessage will get the desired result.

A clever developer might not like this, because she wants to avoid allocating the EventArgs unless there actually are handlers to call. Clever optimization, but it breaks the pattern.

Make sense?

Happy hacking.