This is the archived version of Roland Weigelt's weblog that ran from 2003 to 2023 at weblogs.asp.net

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.

 

7 Comments

  • I agree. I would add that because it isn't "easily discoverable" it becomes easily forgotten. How to program events is the thing I forget the most when writing C# code and I usually end up having to review the recommendations.

  • I agree. My discomfort with the current event syntax is that it's hard to give correct names. Delegates are both objects and functions, should they have the name of an object or the name of a function?



    Having an Invoke method or a raise/trigger keyword would definitely be better nothing.

  • I think this may be why VB.NET has the simplified "no delegates required" event syntax.



    It seems like you guys are wanting RaiseEvent / AddHandler / RemoveHandler.



    Would you like some background compilation to go with that? :D

  • I've always hated that null test for events. The event handler should take care of it for you.

  • Amen. And it would also take care of the fact that if I follow MS' own event samples from 1.1, I've now got to change 20+ different chunks of code to fix the thread safety issue whereas if that functionality had been encapsulated in a built-in keyword, MS could fix it once, and I could fix my code by hotfixing the CLR/BCL

  • I agree completely. This is one of frustrating aspects when I'm writing C# applications. The VB.NET designers thought of this and created the RaiseEvent keyword.



    With RaiseEvent there is no need to test for null references - this is handled by the compiler. Neither is there any need to explicity create a delegate for the event. This feels better to me, as Roland said the compiler should handle it.



    Public Event ShuttingDown (cancel as Boolean)



    Public Sub OnShuttingDown (ByRef cancel as Boolean)

    RaiseEvent ShuttingDown (False)



    End Sub

  • Check out Trolltech's QT.

Comments have been disabled for this content.