2

I know about C# events and delegates. I find them incredibly useful for event-driven sub-systems. One thing I don't understand, however, is why all the .NET documentation for events uses a very specific pattern for delegates:

public class Foo
{
    public class CustomEventArgs : System.EventArgs
    {
        ...
    }

    public delegate void CustomEventHandler(object sender, EventArgs e);
    public event CustomEventHandler CustomEvent;

    public void OnCustomEvent(int x, float y, bool z)
    {
        if (CustomEvent != null)
            CustomEvent(this, new CustomEventArgs(x, y, z));
    }
}

I find this method of declaring and using delegates incredibly unsecure and clunky compared to using custom delegates. For example:

public class Foo
{
    public delegate void CustomEventHandler(Foo foo, int x, float y, bool z);
    public event CustomEventHandler CustomEvent;

    public void OnCustomEvent(int x, int y, int z)
    {
        if (CustomEvent != null)
            CustomEvent(this, x, y, z);
    }
}

Besides the obvious advantage of "It's the pattern most familiar to .NET programmers", I fail to see any other practical advantages to using delegates with the void CustomEventHandler(object sender, CustomEventArgs e) signatures as opposed to custom delegates. Mainly, you have the following advantages with custom delegates:

  1. You can guarantee the sender is of a specific type.
  2. You don't need a whole new class just to pass the event data, leading to messier code and name pollution

I would be interested to know if there are any other advantages to using the .NET pattern for events and delegates that I might be missing.

Edit:

Mainly as a response to @MainMa, I wanted to give a more concrete example. The example is a simple representation of a character with a concept of health and death. The character throws events when its hurt, or dies. Consider the scenario below, using custom delegates:

public class Character
{
    public delegate void DeathEventHandler(Character character);
    public delegate void HurtEventHandler(Character character, float damage);

    public event DeathEventHandler DeathEvent;
    public event HurtEventHandler HurtEvent;

    public bool IsDead { ... }

    public void ApplyDamage(float damage)
    {
        ...
        OnHurt(damage);
        ...
        if (IsDead)
            OnDeath();
    }

    public void OnHurt(float damage)
    {
        if (HurtEvent != null)
            HurtEvent(this, damage);
    }

    public void OnDeath()
    {
        if (DeathEvent != null)
            DeathEvent(this);
    }
}

To me, this is simple. It separates concerns, and enforces encapsulation. A handler that is only concerned about the death of a character doesn't need to know how much damage was applied to the character before death. So on and so forth.

Now compare this to the .NET pattern:

public class Character
{
    public class CharacterHurtEventArgs : EventArgs
    {
        public float Damage { ... }
        public bool Dead { ... }
    }

    public event EventHandler<CharacterHurtEventArgs> HurtEvent;
    public event EventHandler<EventArgs> DeathEvent; // No args ...?

    // ... The rest is the same story as above more or less ...
}

Using this, I've just added an extra class. The handler still needs different delegates for these events, and there is no guarantee that the object which died was a character. Anything could throw this.

Of course, going by @MainMa's suggestion, we could change this into:

public class Character
{
    public class HealthConditionEventArgs : EventArgs
    {
        public float Damage { ... }
        public bool Dead { ... }
        public float Hitpoints { ... }
        ...
    }

    public event EventHandler<HealthConditionEventArgs> HealthConditionChangedEvent;

    public void ApplyDamage(float damage)
    {
        ...
        HealthConditionChangedEventArgs e = new HealthConditionChangedEventArgs();
        ...
        OnHealthConditionChanged(e);
        ...
    }

    public void OnHealthConditionChanged(HealthConditionEventArgs e)
    {
        if (HealthConditionChangedEvent != null)
            HealthConditionChangedEvent(e);
    }
}

This is slightly neater, but it eliminates separation of concerns. Now if the handler only cares about death, it will have to receive all hurt events. Of course, we could have separate events taking in the same event args, but you'd still be passing around the entire character health condition as part of the event. Again, if you care only about death, there is no need for the handler to know any more than the fact that the character died.

In my opinion, the first example, however, resolves all these encapsulation issues. Discrete delegates for discrete events, with only the necessary information passed in each event. No new classes either.

  • what you are suggesting isn't neater, it limits flexibility. I suggest you take a look at the WinForms API. There are about 254325+ different kinds of events there, and sometimes you just want to register a single handler function to about 20 of these (especially common with DataGridView), because you care that something, somewhere happened, not the specific details of exactly what. – AK_ Feb 7 '15 at 10:54
  • @AK_, That's very fair and valid, which is why I think it depends a bit on the situation the events are used in. When talking about large infrastructures such as WinForms API, now it makes sense why you'd have this and give the user the flexibility for their delegates. But in much smaller sub-systems in which you want to enforce a very strict method of doing things, it makes very little sense. Sometimes limiting flexibility isn't a bad thing. – Zeenobit Feb 7 '15 at 18:46
  • Sometimes it is, but usually not with events. when you design a component to expose events, you usually don't care what your consumers do. You are basically saying: "These are things that can happen in my component. if you want i can notify you when they happen". but you don't care what happens after you triggered the event. If you care about what the handler does, and how, you should probably be using callbacks instead of events. – AK_ Feb 7 '15 at 19:19
  • another thing... lately it's very popular to process event as a stream of data (Reactive Extensions, F# events), there the polymorphic EventArgs allows you to process events as if you are transversing a polymorphic collection... – AK_ Feb 7 '15 at 19:26
6

Let's imagine an example of a presentation layer. The layer has mouse interactions: every time the user moves a mouse, an event telling which mouse moved (imagine the user may have multiple mice) and how the mouse moved can be raised.

With your approach, the draft code would be:

public delegate void MouseMovedHandler(
    PresentationLayer l, Guid mouseId, int fromX, int fromY, int toX, int toY);

It will obviously be quickly refactored into:

public delegate void MouseMovedHandler(
    PresentationLayer l, Guid mouseId, Coord from, Coord to);

But then, you'll find that you want to know, for instance, the distance between from and to. So you'll create a Line class, defined by two points, and your delegate will become:

public delegate void MouseMovedHandler(PresentationLayer l, Guid mouseId, Line motion)

Better. Now, what if we want to be able to make a difference between a simple line and a motion of a specific, identified mouse? Right, we'll end up creating the class:

class MouseMotion : Line
{
    public Mouse Mouse { get; }
    public Line Motion { get; }
    ...
}

public delegate void MouseMovedHandler(PresentationLayer l, MouseMotion motion)

Now, there is a location in your app where you need to track every possible input: mouse motion, mouse button clicks, keyboard strokes. You need that because your application starts background processing when the user is not doing anything, but should stop it and free the resources to the UI if the user is using your app.

This means that now, you need something common between MouseMotion, MouseButtonChange and KeyboardStroke. Something like IEvent. Or wouldn't it be easier to simply use EventArgs?

So you end up with:

class MouseMotionEventArgs: EventArgs { ... }
public delegate void MouseMovedHandler(PresentationLayer l, MouseMotionEventArgs motion)

Except that you could avoid the delegate, as explained at the beginning of the answer. You can now either stick with the delegate in order to have your strongly typed PresentationLayer you probably don't need most of the time, or replace it with a lousy object by using EventHandler<MouseMotionEventArgs>.


The previous version of the answer, for the sake of completeness:

Is there a reason you create a CustomEventHandler instead of using EventHandler<CustomEventArgs>?

With the generic EventHandler<T>, the code starts looking like this:

public class Foo
{
    public event EventHandler<3DPointChangedEventArgs> CustomEvent;

    public void OnCustomEvent(3DPoint point)
    {
        if (CustomEvent != null)
        {
            CustomEvent(this, (3DPointChangedEventArgs)point);
        }
    }
}

Now, compare it to the code you suggested:

Talking specifically about name pollution, having *EventArgs classes is not that bad. Those names are usually very clear and express the intention of the author and cannot be confused with other business objects.


Answering your last edit, there are three points to consider:

  1. It is not necessarily useful to separate the death from the health. The most basic representation of health could be a number; for instance, 0.0 can represent death and 100.0 can correspond to the full health.

    Of course, a number would better be replaced by Health class which can then contain business rules which can be modified through time. For example, if it is a game, further features may include additional health slots, making it possible to have, say, 120% of health.

    This may create a performance issue: one event less means that if something is listening to the death events, it will now be notified about all the health change events. I wouldn't be concerned about that early, and wait until it becomes an actual bottleneck. After all, what if the performance becomes even better, now that we don't have a condition in ApplyDamage()?

  2. If the author decides to keep both events, death event can use EventArgs, making it possible to use EventArgs.Empty. This makes one delegate less to write.

  3. Who listens to the event? Let's imagine the world listens to it in order to remove objects a few minutes after their death (which makes sense, since objects may not be aware of the world, and so won't be able to remove themselves from it).

    This also means that we don't really care if the dead object is a Character or a Trebuchet or a WarShip or a Horse : Animal. This means that it is OK to lose types when transmitting the information through the event: the removal will be the same for a paladin or a sheep.

Here's the final result which applies the point 1 and 3:

public class Health
{
    private double value;

    public Health(double value) { ... }

    public void ApplyDamage() { ... }
}

public class Character
{
    public event EventHandler<HealthEventArgs> HealthChanged;

    public Health Health { get { return this.health; } }

    public void ApplyDamage(double damage)
    {
        ...
        this.health.ApplyDamage(damage);
        ...
    }

    public void OnHealthChange(float damage)
    {
        if (HurtEvent != null)
        {
            HurtEvent(this, damage);
        }
    }
}

The events handling can then be done like this, making abstraction of the type of the world object being removed:

public class World
{
    public void MarkAsDead(object worldObject)
    {
        // Wait for five minutes.
        this.elements.Remove(worldObject);
    }
}
  • The x, y, z parameters were just an example, not necessarily a point type. I should've used a better example. :) But even then, in the .NET pattern, I would have to create an EventArgs class, so it would be longer, defeating your first point. I didn't know about the generic EventHandler type though. That's useful. – Zeenobit Feb 6 '15 at 18:28
  • @Zeenobit: 3DPoint was also just an example. It could have been Product, Customer or a Dog. Creating 3DPointChangedEventArgs is not that annoying: it's just a basic class with a cast (or a specific constructor) inside, and a very explicit name, making it not that much of a pollution as you think. – Arseni Mourzenko Feb 6 '15 at 18:43
  • @Zeenobit: I rewrote the answer by using a concrete illustration. I hope it's easier to understand now. – Arseni Mourzenko Feb 6 '15 at 19:02
  • Thank you. Your example makes a lot of sense for such a broad system. But I also updated my question with a more concrete example. I'd be interested to hear your feedback. – Zeenobit Feb 6 '15 at 20:32
  • @Zeenobit: I edited my question to discuss your latest code examples. – Arseni Mourzenko Feb 6 '15 at 22:26
0

This is the convention adopted by Winforms, and also some other event handlers in the .NET ecosystem. It provide two things: the object that caused the event, and some specific event arguments.

One of the reasons it is done this way is because events can be multicast; you can have one event handler handling multiple events, or one event being broadcast to several handlers. To provide flexibility, the bare object is sent to the event handler, in case we don't actually know what type the object raising the event is. This convention was created prior to the introduction of generics.

Note that the .NET framework provides a generic version of the Event Handler delegate since .NET 2.0, which provides compile-time type safety.

  • 1
    It is part of the .net class library design guidelines, and not specific to windows forms. – Frank Hileman Feb 6 '15 at 19:13
0

I cannot comment yet, but these generics are also valid:

private event Action SomeEvent1;
private event Action<int> SomeEvent2;
private event Func<int, int, int> SomeEvent3;

This provides an ambiguously un-descriptive template if auto-generated:

int CLASSNAME_SomeEvent3(int arg1, int arg2) //arg what? argh!!
{
}

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.