views:

363

answers:

2

So I have started playing around with FxCop lately and one thing I've noticed is it insists that any method attached to an event should be in the form

void Callback(object sender, EventArgs args) { ...}

and to be attached with

MyObject.Event += new EventHandler(Callback);

Now that was all well and good back in the .Net 1.1 days but since 3.5 I've been finding it far easier and more intuitive to just make the event call of type Action or one of its generics, and write the method exactly as I would if it were being called explicitly; none of that object sender or EventHandler cruft.

As a matter of point I think it is a flat out design imperative. If you design a method differently for an event callback that means the method at least implicitly has some information about its invocation - thats a major no-no!

I'm fully willing to accept that I might be missing something. What are you guys' thoughts on this, Is FxCop in the wrong or am I?

A: 

I don't think FxCop has been updated in a good long while now; have you tried it with the VS2008 Code Analysis tools (the successor to FxCop)?

technophile
+1  A: 

You should follow the convention.

  1. Use the generic EventHandler<T>, where T is or derives from EventArgs. Hook up the event with

    MyObject.SomeEvent += new EventHandler<EventArgs>(SomeMethod);

  2. The event handler method should return void (makes no sense to return something to an event raiser), and the convention to take the sender object with data in the event args should be followed.

Reasons for the "crap" (sender and eventargs)

  • Convention
  • Extensibility is more easily accomplished (both the the class that raises the event and the class that handles it)
  • Sometimes you want to know who sent the event.
  • Any/all data can be encapsulated in the event arguments.
  • You can use the same event handler for many events.

The pattern continues further. You should also raise your event "SomeEvent" in a protected method named OnSomeEvent() so that derivers of your class can do things like suppress events, raise them in a thread-safe manner, raise them on a UI thread, raise them with timeouts or exception-protection, log event raising processes, etc.

Hey, it's not a perfect pattern (maybe sender could have been put into the event args) but nearly all .Net code follows it, and the framework code always follows it. Why not follow as well.

Bill
Convention - So? Extensibility - not all that much easier at all. Knowing the caller - Can be useful, in which case it can be an argument (strongly typed) it should not be mandatory. It just seems like this exist because at the time they invented it there were no generics
George Mauer
That probably is the reason it exists. That doesn't necessarily mean that the advent of generics makes it useless or bad, though.
technophile