.NET Implement IEnumerator with nullable reference types

Question

Since the release of C# 8.0, I've really been enjoying the 'void safety' with nullable reference types. However, while tweaking a library of mine to support the new feature, I stumbled upon a 'problem' to which I really couldn't find an answer anywhere. I've looked at Microsoft's release notes and the .NET source code but no luck.

TL;DR: the question is essentially whether or not an IEnumerator<T>'s Current property should be declared as a nullable reference type.

Suppose the following implementation of IEnumerator<T>:

public class WebSocketClientEnumerator<TWebSocketClient> : IEnumerator<TWebSocketClient> where TWebSocketClient : WebSocketClient
{
    private WebSocketRoom<TWebSocketClient> room;
    private int curIndex;
    private TWebSocketClient? curCli;

    public WebSocketClientEnumerator(WebSocketRoom<TWebSocketClient> room)
    {
        this.room = room;
        curIndex = -1;
        curCli = default(TWebSocketClient);
    }

    public bool MoveNext()
    {
        if (++curIndex >= room.Count)
        {
            return false;
        }
        else
        {
            curCli = room[curIndex];
        }

        return true;
    }

    public void Reset() { curIndex = -1; }

    void IDisposable.Dispose() { }

    public TWebSocketClient? Current
    {
        get { return curCli; }
    }

    object IEnumerator.Current
    {
        get { return Current; }
    }
}

And suppose the following code to consume the enumerator:

public class WebSocketRoom<TWebSocketClient> : ICollection<TWebSocketClient> where TWebSocketClient : WebSocketClient
{
    // ...

    public void UseEnumerator()
    {
        var e = new WebSocketClientEnumerator<TWebSocketClient>(this);
        bool hasNext = e.MoveNext();
        if (hasNext)
        {
            WebSocketClient c = e.Current; // <= Warning on this line
        }
    }

    // ...
}

The code, as it is, will generate a warning because, obviously, the return type of WebSocketClientEnumerator<TWebSocketClient>.Current is a nullable reference type.

The IEnumerator interface is designed in such a way that one 'should' call the IEnumerator<T>.MoveNext() method to know beforehand if the enumerator has a next value, thereby accomplishing some kind of void safety but obviously, to the eyes of the compiler this means nothing, calling the MoveNext() method doesn't intrinsically guarantee that the enumerator's Current property is not null.

I would like my library to compile without warnings though and the compiler won't let me leave this.curCli with a null value in the constructor if it is not declared as a nullable reference type and if it is declared nullable then the 'burden' of checking for a null reference is transferred to the client of the library. Granted, enumerators are generally consumed trough foreach statements hence it's mostly handeled by the runtime and it's probably not that big of a deal. It is true that semantically speaking, it makes sense for an enumerator's Current property to be null because there might be no data to enumerate, but I really do see a conflict between the IEnumerator<T> interface and the nullable reference type feature. I'm really wondering if there's a way to make the compiler happy while still maintaining the functionality. And also, what are the conventions in some other languages featuring some void safety mechanisms?

I realize this is kind of an open-ended question but I still think it's appropriate for SO. Thanks in advance!

Answer 1

I would definitely suggest it should be declared as the not-null version. The documentation for Current states that the behavior is defined in the cases where curCli is actually null. I'd argue that anyone reading Current in those cases has a bug in their code, and it would be best to surface that bug via an exception... which is really easy to do:

public TWebSocketClient Current => curCli ??
    throw new InvalidOperationException("Current should not be used in the current state");

You might want to set curCli to null when you return false as well, so that the exception is also raised if code accesses Current after exhausting the enumerator.

At that point, I think your code is better than the code generated by the compiler for yield return, which doesn't throw exceptions.