Is it bad practice to have Events in interfaces? c#

Question

I'm trying to use interfaces more, and often I find myself putting events into the interface. It feels strange, because I'm leaving up to the implementer to know when to raise these events within their implementation.

For example, I have an ILevellable interface, which simply states an object should have xp, a level and a max level, and some methods which modify these values. Classes referencing ILevellable would probably want to know when it levelled up, so I add an Action for OnLevelUp... but the implementation might not put that in the correct place.

public interface ILevellable
{
    int Level { get; }
    int MaxLevel { get; }
    int XP { get; }
    event Action<bool> OnXPChanged;
    event Action<ILevellable> OnLevelUp;
    event Action OnMaxLevelReached;

    void LevelUp(int newLevel);
}

Am I supposed to trust that users implementing this interface will know where to implement this? I would assume not. Especially because some events might have arguments which, again, the user might have no idea what it is supposed to represent.

I appreciate any guidance! I am new to working like this.

Thanks!

This might get flagged as "opinion-based", which might be all the answer you need. I think this is okay if the events have *very good* commenting describing, for example, what the `bool` in `Action` is. Alternately, for an interface released into the wild, I would probably use `EventHandler` and `EventArgs`, which force a very formal definition of the event handler. I consider them overly long-winded for internal code, but.... — zzxyz, Sep 05 '18 at 01:57
And note that it's *almost* as easy to implement properties and methods within an interface that are completely inscrutable to someone trying to implement the interface. (For example, what should my implementation of `LevelUp`do if a level lower than the current level is passed in?) — zzxyz, Sep 05 '18 at 02:01
@zzxyz yeah I was thinking having EventArgs or just some data object could be a good solution as well... Thanks for your "opinion" :D I'll comment my stuff now... — Sebastian King, Sep 05 '18 at 02:05
You are not talking about delegates here but about events. Consider editing your question =) — taquion, Sep 05 '18 at 02:17
@SebastianKing - Keep in mind that interfaces only describe the interface - they **do not** describe the behaviour. That's not just for events but for all of the properties and methods you expose too. Typically when you pair an interface and a set of unit tests only then you specify behaviour too. — Enigmativity, Sep 05 '18 at 02:48
@Enigmativity - I've heard that guideline before, although Microsoft certainly seems to break it quite often. Well...obviously not in the interface itself, but in the documentation of the interface. Is that the distinction? Or is the guideline that the interface just flat-out shouldn't be associated with specific behavior? If it's the latter, I would like to read the reasoning behind it. (Because, for example, `//this method does not throw exceptions` seems like a perfectly reasonable rule/doc in an interface) — zzxyz, Sep 05 '18 at 19:13
@zzxyz - Behaviour might be implied by the interface, but there's no way to specify behaviour explicitly. You either have to describe the behaviour or provide unit tests - but neither of these are enforced by C#. — Enigmativity, Sep 06 '18 at 00:34

sjb-sjb · Accepted Answer · 2018-09-06T01:53:15.560

It's fine to define events in your interfaces. However it is consistent with convention to have both a 'sender' and 'event args' passed into the event. TypedEventHandler is an easy way to do it. For example:

using Windows.Foundation;

public struct LevellableChange {
    public LevellableChange( int dl, int dxp) 
    { 
         this.ChangeInLevel = dl;
         this.ChangeInXP = dxp;
    }
    int ChangeInLevel { get; }
    int ChangeInXP {get;}
}

public interface ILevellable
{
    int Level { get; }
    int MaxLevel { get; }
    int XP { get; }
    event TypedEventHandler< ILevellable, LevellableChange> Changed;
}

Then the standard approach to implementing them would be like this:

public class Levellable: ILevellable
{
    public event TypedEventHandler<ILevellable, LevellableChange> Changed;

    public int Level {
        get {
            return this._level;
        }
        private set {
            if (value != this._level) {
                int oldLevel = this._level;
                this._level = value;
                this.Changed?.Invoke(this, new LevellableChange(value - oldLevel, 0));
            }
        }
    }
    private int _level;

    // similar for XP. 
}

One other comment, it is useful to have a class implementing INotifyPropertyChanged, and then get in the habit of deriving from it. Together with a snippet in Visual Studio it makes things a lot easier. It's true that for the example above, INotifyPropertyChanged would not be enough since it doesn't give the change to the previous value.

Note, events can be the cause of memory leaks. If object A is listening to an event implemented by object B, then even if A is no longer needed it will be held in memory as long as B is referenced / being used. To avoid this you need to either: unsubscribed A from the event; or make sure that the lifetime of B is not much longer than the desired lifetime of A; or use weak event handlers. — sjb-sjb, Sep 11 '18 at 02:09

score 0 · Answer 2 · edited Sep 07 '18 at 09:09

First of all you are not having delegate types in your interface definition but events. Check this answer by Jon Skeet to see the difference between delegates and events. After saying that it is totally fine to have events in your interface, even .Net does it, consider for instance the famous INotifyPropertyChanged interface. It just defines an event...

Secondly:

Am I supposed to trust that users implementing this interface will know where to implement this?

When you define an interface you really do not care about implementers as much as about consumers, and so, for a consumer that sees your contract definition it is useful to know that he/she can susbscribe to an event named OnLevelUp. Let's say I am indeed consuming your interface, I do not know how or when you trigger OnLevelUp event, and I should not really care, because you have given to that event a very semantic and meaningful name, so I can pretty much be sure that it will be triggered when the ILevellable's Level property is increased.

Oh my mistake. Ok I understand what you're saying. I suppose I was hoping there would be some kind of 'safe' workaround but I suppose it really does just come down to naming. Thanks for the link as well. I shall read it :). — Sebastian King, Sep 05 '18 at 02:29

Is it bad practice to have Events in interfaces? c#

2 Answers2