Raising C# events doesn't feel right (and seems to have problems too)

[This blog post was inspired by reading “Events, Delegate and Multithreading” by Brad Abrams.]

I'm pretty sure the language designers thought about the whole thing way longer than I did, but from the very beginning raising events simply didn't "feel right". I’m not talking about the pattern of using “On…” methods — while that was something I (with an extensive Javascript background) had to get used to, it was a typical case of “what? whose idea was that? hmm, that’s not so dumb after all… hey, good idea!”.

No, it’s the way of actually raising the event inside the “On…” method. In the blog post mention above Eric Gunnerson is quoted “We have talked about adding an Invoke() keyword to the language to make this easier, but after a fair bit of discussion, we decided that we couldn't do the right thing all the time, so we elected not to do anything”. Well, but that’s exactly what I’m missing: a keyword. While I don’t miss much from VB6 (ok, maybe the command window), I really miss the good ol’ RaiseEvent.

When you look at source code, keywords are something that catch your eye. While you are bombarded with lots and lots of words (names of classes, variables, methods, etc. ), keywords like “delegate”, “event”, “throw”, “override” (man, I’m thankful for that one!) stand out — not only because of the coloring inside the editor, but also because of the different syntax compared to lines and lines of “variable = Method(parameter)”.

For example throwing an exception:

throw new FooException("bar");

It says “throw” and you can virtually see some referees throwing a yellow flag.

If I declare a delegate “FooHandler” and an event "Foo"

public delegate void FooHandler(object sender, FooArgs e);
public event FooHandler Foo;

then one can argue the overall approach (is it too complicated yes/no?), but nevertheless it’s quite clear what the code is trying to tell me: This is a delegate, and this is an event, and there seems to be some connection between those two (because FooHandler appears in both).

But then comes the big letdown: the code for raising the event. Until recently I would have used

protected virtual void OnFoo(FooArgs e)
{
 if (Foo != null)
 Foo(this, e);
}

Now I’ve read the recommendation to use

protected virtual void OnFoo(FooArgs e)
{
 FooHandler handler = Foo;
 if (handler != null)
 handler(this, e);
}

Sorry, but this doesn’t survive my personal test for “is the important stuff easily discoverable”: Step back from the monitor, squint your eyes until things look kind of blurry, and then collect all information you can get from what you see with a quick look. For fun, try it with the code example. While “throw new blah blah” and “public event blah blah blah” give you a rough idea of what’s happening, you have to look again to find out what “protected virtual blah handler handler blah handler null blah this” is all about. To avoid misunderstandings: This is not about understanding a programming language without learning it. What I’m talking about is browsing code really fast, looking at each line only fractions of a second, and not actually thinking about the code, but just matching some basic patterns inside your brain — stuff we’re all doing automatically when scrolling up and down while editing text.

Ok, this post is longer than it should be, so let’s cut this whole thing short and get to the point:

Comparing raising an event with the way how to throw an exception, and keeping in mind that the official wording (according to the Design Guidelines for Class Library Developers) is that exceptions are "thrown" and events are "raised", I would personally prefer:

protected virtual void OnFoo(FooArgs e)
{
 raise Foo(this, e);
}

What should the “raise” keyword do internally? Let me play the role of the unfair customer here: I don’t care, just make it work. I want to raise an event and that should work without the danger of a race condition or whatever.