Even "IsNullOrEmpty" checks give "Possible multiple enumeration of IEnumerable" warnings

Question

There's a question on SO about "possible multiple enumerations" already, but this question is more specific.

Please consider the following method, which takes an IEnumerable<string> as input and executes a given method against each of its elements:

public static bool SomeMethod(IEnumerable<string> enumerable)
{
    if (enumerable.IsNullOrEmpty())
    {
        // throw exception.
    }
    else
    {
        return (enumerable.All(SomeBooleanMethod));
    }
}

In the code above, IsNullOrEmpty is just an extension method which runs

return (!ReferenceEquals(enumerable, null) || enumerable.Any());

The problem is that ReSharper is warning me about "Possible multiple enumerations of IEnumerable", and I really don't know if this can actually be a problem or not.

I understand the meaning of the warning, but what could you really do in this situation if you really need to check and throw exception in case of nullity or emptyness?

Why do you want this method to throw if passed an empty sequence? The semantics of "Do something to every member of this (empty) sequence" are perfectly obvious, aren't they? Also, is there any particular reason why you use `ReferenceEquals()` rather than just `== null`? — AakashM, Aug 30 '11 at 13:54
It indeed is not, but now imagine this was a constructor. If I cannot construct an object from an empty sequence, should I not throw an exceptio? — User, Aug 30 '11 at 13:56
Sure, if there really is nothing you can do; but *in general* I would say empty sequences should be treated as just as good as any non-empty sequences. `List<>` is perfectly happy to construct from an empty sequence, for example. Of course I am only talking in generalities; you know the *details* of your situation. — AakashM, Aug 31 '11 at 14:06

score 30 · Answer 1 · answered Aug 30 '11 at 13:51

30

It means that you are (partially) iterating over the IEnumerable more than once: first in your call to Any() (which needs to at least initialize an iteration to see if the enumerable returns any elements), and a second time in All (which iterates from the beginning).

The reason ReSharper warns you about this is that enumerating over an enumerable may cause side effects, and unintentionally iterating twice may trigger the side effects twice, which may or may not be desirable.

answered Aug 30 '11 at 13:51

tdammers

20,353
1
39
56

could you please explain the side effects or any examples would be delightful. – Beytan Kurt May 29 '12 at 13:31
13

Your enumerable could, for example, read bytes from a network stream (which would constitute a side effect). If you enumerate once, all is fine, and the stream is read beginning to end in one go. If, however, you abort the first iteration and then iterate again, even though the underlying network stream cannot seek backwards, you will most likely get undesired behavior. – tdammers May 29 '12 at 18:54
It's worth noting that Resharper doesn't know about the call to `Any()`. All it knows is that `SomeMethod()` passes the enumerable to two other methods(`IsNullOrEmpty()` and `All()`), which may or may not enumerate it. – bornfromanegg Nov 01 '19 at 11:59

score 8 · Answer 2 · answered Sep 02 '11 at 09:09

As @tdammers identifies, the "multiple enumerations" referred to are the two enumerations required by Any and All. Since you want to reject an empty sequence, the best I can come up with is:

public static bool SomeMethod(IEnumerable<string> enumerable)
{
    if (enumerable == null)
        throw new ArgumentNullException();

    // Manually perform an All, keeping track of if there are any elements
    bool anyElements = false;

    bool result = true;

    foreach (string item in enumerable)
    {
        anyElements = true;
        result = result && SomeBooleanMethod(item);

        // Can short-circuit here
        if (!result)
            break;
    }

    if (!anyElements)
        throw new ArgumentException();    // Empty sequence is invalid argument

    return result;
}

The _possible_ multiple enumerations referred to are not `Any` and `All`. They are `IsNullOrEmpty` and `All`. — bornfromanegg, Nov 01 '19 at 11:56

score 0 · Answer 3 · answered Nov 01 '19 at 11:46

Whilst the other answers here are correct in that you are enumerating twice (and the potential harm in doing so), they are both (subtly) incorrect as to why you are getting the warning.

Resharper is not warning you because you are calling Any() and All(). It is warning you because you are calling IsNullOrEmpty() and All(). In fact, Resharper doesn't even know you are calling Any().Try removing it - you will find you still get the warning.

This is because Resharper has no idea what happens to an enumerable which is passed to another method. Maybe that other method enumerates it, maybe not. But you are passing the enumerable to two methods, so maybe they both enumerate it, so maybe it is enumerated twice. Hence the warning, "Possible multiple enumeration'.

This is subtle but important. In your case, the warning was useful, because you were enumerating twice. But maybe your extension method doesn't enumerate the enumerable and you know the warning can be ignored. In this case, Resharper provides you with the NoEnumeration attribute in Resharper's Code Annotations.

This allows you to tag an enumerable, as in the following made up method:

public static bool IsNull<T>([NoEnumeration]this IEnumerable<T> enumerable)
{
    return enumerable is null;
}

Even "IsNullOrEmpty" checks give "Possible multiple enumeration of IEnumerable" warnings

3 Answers3

Linked