0
votes

ReSharper says "Possible multiple enumeration of IEnumerable" on this code:

public static IEnumerable<T> Each<T>(this IEnumerable<T> @this, Action<T> action)
{
    foreach (var i in @this)
        action(i);

    return @this;
}

But I just return @this, I don't do anything else with it... is it warning me of possibility for additional enumeration once the function returns, or I'm missing something here ?

2
this was answered before, try this: stackoverflow.com/questions/8240844/…Wasafa1
Yes, it's because it knows you have enumerated it once, and the caller could enumerate it again.Matthew Watson
@Wasafa1 - already saw that thread, TL/DR. My question is short and accurate.Tar
It really depends on what you expect to happen. Consider the code SomeSource.Each(MyAction).FirstOrDefault(). What do you expect to happen? In your case the Each method performs MyAction on the entire list even though only the first item was requested. Also, if you dispose of the enumerator before it is emptied (eg use "break;" in a foreach loop), the entire set has been passed as parameter to an invocation of MyAction.Tormod

2 Answers

3
votes

But I just return @this, I don't do anything else with it...

Yes, but the caller will probably also enumerate it...

Anyway, using an iterator block like you did in your answer is better, because:

  • it avoids multiple enumeration
  • it maintains deferred execution (i.e. the source sequence won't be enumerated until you start enumerating the result of Each)
1
votes

This avoids the warning and I assume that it's more efficient:

public static IEnumerable<T> Each<T>(this IEnumerable<T> @this, Action<T> action)
{
    foreach (var i in @this)
    {
        action(i);
        yield return i;
    }
}

Could someone verify that it is indeed more efficient (doesn't enumerate twice ?) ?