22

There's a standard pattern for events in .NET - they use a delegate type that takes a plain object called sender and then the actual "payload" in a second parameter, which should be derived from EventArgs.

The rationale for the second parameter being derived from EventArgs seems pretty clear (see the .NET Framework Standard Library Annotated Reference). It is intended to ensure binary compatibility between event sinks and sources as the software evolves. For every event, even if it only has one argument, we derive a custom event arguments class that has a single property containing that argument, so that way we retain the ability to add more properties to the payload in future versions without breaking existing client code. Very important in an ecosystem of independently-developed components.

But I find that the same goes for zero arguments. This means that if I have an event that has no arguments in my first version, and I write:

public event EventHandler Click;

... then I'm doing it wrong. If I change the delegate type in the future to a new class as its payload:

public class ClickEventArgs : EventArgs { ...

... I will break binary compatibility with my clients. The client ends up bound to a specific overload of an internal method add_Click that takes EventHandler, and if I change the delegate type then they can't find that overload, so there's a MissingMethodException.

Okay, so what if I use the handy generic version?

public EventHandler<EventArgs> Click;

No, still wrong, because an EventHandler<ClickEventArgs> is not an EventHandler<EventArgs>.

So to get the benefit of EventArgs, you have to derive from it, rather than using it directly as is. If you don't, you may as well not be using it (it seems to me).

Then there's the first argument, sender. It seems to me like a recipe for unholy coupling. An event firing is essentially a function call. Should the function, generally speaking, have the ability to dig back through the stack and find out who the caller was, and adjust its behaviour accordingly? Should we mandate that interfaces should look like this?

public interface IFoo
{
    void Bar(object caller, int actualArg1, ...);
}

After all, the implementor of Bar might want to know who the caller was, so they can query for additional information! I hope you're puking by now. Why should it be any different for events?

So even if I am prepared to take the pain of making a standalone EventArgs-derived class for every event I declare, just to make it worth my while using EventArgs at all, I definitely would prefer to drop the object sender argument.

Visual Studio's autocompletion feature doesn't seem to care what delegate you use for an event - you can type += [hit Space, Return] and it writes a handler method for you that matches whatever delegate it happens to be.

So what value would I lose by deviating from the standard pattern?

As a bonus question, will C#/CLR 4.0 do anything to change this, perhaps via contravariance in delegates? I attempted to investigate this but hit another problem. I originally included this aspect of the question in that other question, but it caused confusion there. And it seems a bit much to split this up into a total of three questions...

Update:

Turns out I was right to wonder about the effects of contravariance on this whole issue!

As noted elsewhere, the new compiler rules leave a hole in the type system that blows up at runtime. The hole has effectively been plugged by defining EventHandler<T> differently to Action<T>.

So for events, to avoid that type hole you should not use Action<T>. That doesn't mean you have to use EventHandler<TEventArgs>; it just means that if you use a generic delegate type, don't pick one that is enabled for contravariance.

Community
  • 1
  • 1
Daniel Earwicker
  • 114,894
  • 38
  • 205
  • 284
  • Be interested to hear a reason from downvoter. – Daniel Earwicker Jul 13 '09 at 16:25
  • I haven't voted either way, but I suspect the downvote was because you've got at least two questions in one here. I don't want to really try to tackle the whole thing myself, but I'd be happy to look at the variance aspect (as I'm writing about it right now). If you could separate the bit which leads to an execution-time failure out into a separate question with a short but complete case to reproduce it, I'd be interested... – Jon Skeet Jul 13 '09 at 16:41
  • 1
    I also dislike the `EventHandler` pattern! The only time I like using it is for UI-related events, just so my events have visual consistency. It's practically aesthetic. – Scott Rippey Dec 07 '10 at 22:47

2 Answers2

7

Nothing, you lose nothing. I've been using Action<> since .NET 3.5 came out and it is far more natural and easier to program against.

I don't even deal with the EventHandler type for generated event handlers anymore, simply write the method signature you want and wire it up with a lambda:

btnCompleteOrder.OnClick += (o,e) => _presenter.CompleteOrder();
George Stocker
  • 57,289
  • 29
  • 176
  • 237
George Mauer
  • 117,483
  • 131
  • 382
  • 612
  • 3
    Don't you have problem unsubscribing to the event with this anonymous function? – Matt Jul 13 '09 at 16:56
  • 2
    I suppose you would, but you shouldn't really be unsubscribing to an OnClick event since that merely is meant to notify your presenter that something happened. If you're subscribing/unsubscribing you probably care more about events raised by non-ui classes, in that case you have full control of event type and can make it an Action<> anyways so anonymous functions aren't necessary. – George Mauer Jul 13 '09 at 17:00
  • @Matt - I write the majority of my event enlisting that way. The need to explicitly unsubscribe is pretty unusual. Essentially if the event source is a longer-lived object (e.g. stored in a static, or the main window of the application) but if the source and sink have the same lifetime, why bother? This is what the GC is for. – Daniel Earwicker Jul 13 '09 at 17:04
  • I agree, it is a very nice syntax, and have used it on occasion myself. But there are occasions when you need to unsubscribe, so I felt compelled to point it out. It is very easy, especially for someone new to event handlers, to forget that they are a reference like anything else, and thus can keep objects alive longer than they should be – Matt Jul 13 '09 at 17:09
  • 1
    I agree Matt, and it is certainly a problem if someone's wiring their UI up haphazardly. But if you're doing it even halfway correctly you're going to have some sort of Presenter/Controller/Mediator class that has the same lifetime as your UI which 99% of the time should be the ONLY thing wired up to your UI. – George Mauer Jul 13 '09 at 17:14
  • Not seeing any advantage to the .NET standard I stopped bothering with the (object sender, EventArgs e) pattern a while ago and turned the warning off in FxCop. – Robert Davis Feb 22 '10 at 21:06
  • @George: I really dislike leaving events subscribed when an object is disposed. To be sure, it's annoying that VB doesn't provide a means of auto-setting all WithEvents fields to null, but leaving handlers wired just seems like senseless sloppiness. – supercat Dec 18 '10 at 03:44
  • @supercat that sounds more like a problem of perception than anything. If object1 is subscribed to object2.SomeEvent the semantic meaning is that "object1 is interested in message `object2.SomeEvent`" If you look at it that way then whether object2 has been disposed of or not is irrelevant. And thankfully we work in a garbage collected language so we don't have to worry about the leaky abstraction – George Mauer Dec 20 '10 at 14:54
  • @George Mauer: If an object is disposed, that should in many cases imply that it's no longer interested in messages from other objects. For example, an iEnumerable may need to receive collection-changed events from the collection being enumerated. Once the enumerator is disposed, such events will be useless, but if not detached they will keep the enumerator alive as long as the collection is alive. If the collection is kept alive forever (not an implausible scenario) and is enumerated thousands of times, that could be a real problem. – supercat Dec 20 '10 at 15:45
  • @George Mauer: There are ways of setting up objects that receive events in such a way that they can safely be garbage-collected even when the 'publisher' remains in scope, but such patterns are fraught with peril. It's better to detach events when an object is Disposed. – supercat Dec 20 '10 at 15:48
  • @supercat For complex use-cases where life-cycle management is an actual concern I would recommend against using a mechanism like .Net events and for something that gives you more control like an event aggregator or observer implementation. – George Mauer Dec 25 '10 at 01:11
  • @George Mauer: It seems the normal paradigm for "observable" objects is to provide events. If an ObservableWidget uses events, any potentially-short-lived object which holds (not necessarily owns) one and and subscribes to its events must unsubscribe from those events when disposed. – supercat Jan 01 '11 at 18:16
  • @supercat - I think you raise valid points and in practice there is usually little enough difference between what you and I are saying so don't think I'm being dismissive. I think I'm getting at something deeper. Consider the message passing paradigm that I proposed. Here the meaning of an event subscription is "send message MethodToInvoke to myObject". If 'myObject' doesn't exist, the message vanishes into the ether. *This is semantically equivalent to regular method invocation*. Observer is a pattern for emulating this mechanism in Java/C#... – George Mauer Jan 02 '11 at 19:46
  • ... and .Net events are a limited language-baked implementation of this pattern. They happen to be an implementation that sacrifice this benefit but that is by no means something that is endemic to Observer. A full fledged implementation has a wide range of options for dealing with object disposal and indeed might not use events at all. This is one of the reasons I recommend it for more complex scenarios. .Net events are great, but they are on implementation of a much richer pattern. – George Mauer Jan 02 '11 at 19:50
  • @George Mauer: Ever since I've learned how events are implemented, I've felt that the implementation of multicast delegates is something of a hack to allow delegates to be used to hold subscription lists, when it might have been better to use some other means to do so (the nicer approaches would require generics, which weren't in 1.0, so multicast delegates may have been the best option at the time). It's interesting to note that the only thing "magic" about the MulticastDelegate type is that it is compatible with Delegate.Remove; otherwise one could combine delegates by simply... – supercat Jan 03 '11 at 16:02
  • ...designing an object which holds two or more delegates and has an "invoke" method which calls them, and creating an ordinary delegate to call that object's "invoke" method. BTW, I wonder what the pros and cons would have been of having delegate types be abstract inheritable types, such that a class which overrode the "Invoke" method could be passed as a delegate. Doing that might have precluded some of the optimizations applied to delegates, but would have eliminated the need to create many redundant objects whose sole purpose is to be invoked by a single delegate. – supercat Jan 03 '11 at 16:16
  • My guess would be that such inheritance might have been impractical before the advent of generics; too bad, since it seems that creating an object with a single method whose purpose is to be invoked by a delegate is a fairly common pattern. – supercat Jan 03 '11 at 16:21
  • Agreed, would have been quite nifty – George Mauer Jan 04 '11 at 14:56
2

I don't like the event-handler pattern either. To my mind, the Sender object isn't really all that helpful. In cases where an event is saying something happened to some object (e.g. a change notification) it would be more helpful to have the information in the EventArgs. The only use I could kinda-sorta see for Sender would be to unsubscribe from an event, but it's not always clear what event one should unsubscribe to.

Incidentally, if I had my druthers, an Event wouldn't be an AddHandler method and a RemoveHandler method; it would just be an AddHandler method which would return a MethodInvoker that could be used for unsubscription. Rather than a Sender argument, I'd have the first argument be a copy of the MethodInvoker required for unsubscription (in case an object finds itself receiving events to which the unsubscribe invoker has been lost). The standard MulticastDelegate wouldn't be suitable for dispatching events (since each subscriber should receive a different unsubscription delegate) but unsubscribing events wouldn't require a linear search through an invocation list.

supercat
  • 77,689
  • 9
  • 166
  • 211