Guidelines for events since non-nullable references

user_10883465 0
Viewed 6910

When working with C# 8 and the new non-nullable references, I realized that events are treated like fields. This means that they will cause a warning 90% of the time since they won't be initialized until someone subscribes to it.

Consider the following event:

public event EventHandler IdleTimeoutReached;

You get the following warning on the line of the constructor declaration.

CS8618 C# Non-nullable event is uninitialized. Consider declaring the event as nullable.

If you make it nullable, the warning disappears of course. However, this looks very weird to me.

public event EventHandler? IdleTimeoutReached;

An alternative would be assigning it a no-op, but this seems even worse.

public event EventHandler IdleTimeoutReached = new EventHandler((o, e) => { });

What's the correct way to handle this situation and get rid of the warning without just disabling it? Are there any official guidelines for this?

c#
2 Answers

You should make the event nullable, because it is indeed nullable. It may look a bit strange but that's the correct way of expressing your intent.

It has always been necessary to check whether an event is null before raising it within the class. In this case nullable reference checking will warn you if you try to invoke it without checking for null.

    public class Publisher
    {
        public event EventHandler? IdleTimeoutReached;

        protected virtual void RaiseIdleTimeoutEvent()
        {
            // `IdleTimeoutReached` will be null if no subscribers; compiler gives warning about possible null reference
            IdleTimeoutReached.Invoke(this, EventArgs.Empty);

            // No compiler warning
            IdleTimeoutReached?.Invoke(this, EventArgs.Empty);
        }
    }

The nullable annotation really only adds value for the class invoking the event. It won't affect callers at all because the += / -= syntax takes care of correctly assigning/removing those delegates.

So you should use:

public event EventHandler? IdleTimeoutReached;

For events with EventArgs, use:

public event EventHandler<IdleTimeoutEventArgs>? IdleTimeoutReached;
user_3939310

The whole point of non-nullable reference types in C# is to obviate the need for writing code that deals with nulls. Null checks make code more verbose and harder to read. By removing the need for always having to do null checking, code is made more concise and readable.

In that spirit, I would recommend that you require a reference to an event handler in the constructor for the type in which your event appears. This will make the compiler happy, and it means that every time your type containing an event is instantiated, there will be an event handler in place to handle your event. This ensures that your type is always used in the way that it is intended to be used.

Now, someone might say that it is better to make the event nullable so that it is optional. My view, however, is that if a type defines an event, it does so for an important reason, and if the user of that type wishes to ignore that important reason and not use the event, then that user can define an empty event handler so that it is made clear in the code that the event is intentionally being ignored.

In the end you get better communication between classes, and you also eliminate the need for null checks, so the benefits are twofold.

The code would look something like this:

class Program
{
    static void Main()
    {
        var myType1 = new MyType(IdleTimeout);
        
        // Or if you don't want a superfluous handler in your code,
        // you can use a Lambda.
        var myType2 = new MyType((sender, e) => {});
    }

    static void IdleTimeout(object? sender, EventArgs e)
    {
        // Event ignored.
    }
}

class MyType
{
    public MyType(EventHandler idleTimeoutReached)
    {
        IdleTimeoutReached += idleTimeoutReached;
    }

    public event EventHandler IdleTimeoutReached;

    public void TimeoutReached()
    {
        // Any required logic goes here.

        IdleTimeoutReached?.Invoke(this, EventArgs.Empty);
    }
}
user_109135970
Related