2

Rider/ReSharper gives me possible multiple enumeration warning on this:

public void ProcessProductCodes(IEnumerable<string> productCodes) {
    if (productCodes.Any()) {
        DoStuff(productCodes);
    }
}

Is it a false positive, or does the Any() function indeed mess up enumeration of collection?

Pang
  • 9,564
  • 146
  • 81
  • 122
  • 2
    The only way to figure out if there's `Any()` element in the enumeration is to start enumerating. While `Any()` stops after at most one element, it still enumerates. Make it so `DoStuff()` operates correctly on an empty collection and there's no problem. – Jeroen Mostert May 09 '20 at 13:35

3 Answers3

4

The IEnumerable interface represents a sequence of items that can be iterated over, but makes no assumptions about the origin of the sequence. For instance, it could be a database query. If that was the case, here you would make 2 calls to the database, one for checking if there is any item in the sequence and another one to pass them to the DoStuff function, which obviously is not optimal performance-wise, and ReSharper is warning you about this.

To avoid this problem, you have 2 different options. If the collection of items is already in memory, you could make it explicit by changing the signature of your function to:

public void ProcessProductCodes(ICollection<string> productCodes) { ... }

If you cannot guarantee this, you can do a .ToList() or .ToArray at the beginning of your function:

public void ProcessProductCodes(IEnumerable<string> productCodes) {
  var productCodesList = productCodes.ToList();
  if (productCodesList .Any()) {
      DoStuff(productCodesList );
  }

ReSharper will do this for you, just select the quick refactoring (usually with Alt+Enter).

Pang
  • 9,564
  • 146
  • 81
  • 122
Tao Gómez Gil
  • 2,228
  • 20
  • 36
  • Well I understand that. But tehnically it is possible to enumerate through collection items just once, by peeking the first element. In case of Database call, I'll do the database call on Any(), peek the first element and start moving the datareader when I'm doing the real enumeration. – Andre Tchernikov May 09 '20 at 13:47
  • 2
    If you first "peek" and then "do the real enumeration", you are doing performing the enumeration twice, that's how `IEnumerable` works. Think of `IEnumerable` not as a collection of items, but as the "instructions to get the items of a collection". Each time you access them, even if you only want the first one, you'll perform the operation that gets them (potentially doing a costly one like a database call). – Tao Gómez Gil May 09 '20 at 13:58
4

You could create a helper method (I've created this as an extension method) to ensure that it's only iterated once:

public static class LinqExtensions
{
    public static bool DoIfAny<T>(this IEnumerable<T> collection, Action<IEnumerable<T>> action)
    {
        var enumerator = collection.GetEnumerator();
        if (!enumerator.MoveNext())
        {
            return false;
        }

        action(CreateEnumerableFromStartedEnumerable(enumerator));
        return true;
    }

    private static IEnumerable<T> CreateEnumerableFromStartedEnumerable<T>(IEnumerator<T> enumerator)
    {
        do
        {
            yield return enumerator.Current;
        }
        while (enumerator.MoveNext());
    }
}

Essentially this will create an enumerator for the collection, and then try to move to the first item. If that fails, then the method is not called and false is returned.

If it's successful, then it will produce a new enumerable which iterates the rest of the source enumerable, yielding its values as it goes. This includes the very first value. This will then be passed to an action delegate and true will be returned.

Usage:

IEnumerable<string> values = new string[] { "a", "b", "c" };
bool delegateCalled = values.DoIfAny(e => DoStuff(e));
Console.WriteLine("Delegate called: " + delegateCalled.ToString());

Try it online

Note that this will only really work if you want .Any() in the sense of "the collection isn't empty". If it is checking for the existence of a specific element, then you'll have to materialise the list first as in Tao's answer.

ProgrammingLlama
  • 36,677
  • 7
  • 67
  • 86
0

Any() (without a predicate) is optimized and does not iterate through the collection. You can see for yourself: https://referencesource.microsoft.com/#System.Core/System/Linq/Enumerable.cs

If you use a predicate version then yes you will iterate.