Handling warning for possible multiple enumeration of IEnumerable

Question

In my code in need to use an IEnumerable lt  gt  several times thus get the Resharper error of  Possible multiple enumeration of IEnumerable    Sample code   public List lt object gt  Foo IEnumerable lt object gt  objects        if  objects    null     objects Any            throw new ArgumentException         var firstObject   objects First        var list   DoSomeThing firstObject               var secondList   DoSomeThingElse objects       list AddRange secondList        return list       I can change the objects parameter to be List and then avoid the possible multiple enumeration but then I don t get the highest object that I can handle    Another thing that I can do is to convert the IEnumerable to List at the beginning of the method          public List lt object gt  Foo IEnumerable lt object gt  objects         var objectList   objects ToList                    But this is just awkward   What would you do in this scenario

User · Answer

I usually overload my method with IEnumerable and IList in this situation.

public static IEnumerable<T> Method<T>( this IList<T> source ){... }

public static IEnumerable<T> Method<T>( this IEnumerable<T> source )
{
    /*input checks on source parameter here*/
    return Method( source.ToList() );
}

I take care to explain in the summary comments of the methods that calling IEnumerable will perform a .ToList().

The programmer can choose to .ToList() at a higher level if multiple operations are being concatenated and then call the IList overload or let my IEnumerable overload take care of that.

User · Answer

The problem with taking IEnumerable as a parameter is that it tells callers  I wish to enumerate this   It doesn t tell them how many times you wish to enumerate       I can change the objects parameter to be List and then avoid the possible multiple enumeration but then I don t get the highest object that I can handle    The goal of taking the highest object is noble  but it leaves room for too many assumptions  Do you really want someone to pass a LINQ to SQL query to this method  only for you to enumerate it twice  getting potentially different results each time    The semantic missing here is that a caller  who perhaps doesn t take time to read the details of the method  may assume you only iterate once - so they pass you an expensive object  Your method signature doesn t indicate either way    By changing the method signature to IList ICollection  you will at least make it clearer to the caller what your expectations are  and they can avoid costly mistakes    Otherwise  most developers looking at the method might assume you only iterate once  If taking an IEnumerable is so important  you should consider doing the  ToList   at the start of the method    It s a shame  NET doesn t have an interface that is IEnumerable   Count   Indexer  without Add Remove etc  methods  which is what I suspect would solve this problem

User · Answer

First off  that warning does not always mean so much  I usually disabled it after making sure it s not a performance bottle neck  It just means the IEnumerable is evaluated twice  wich is usually not a problem unless the evaluation itself takes a long time  Even if it does take a long time  in this case your only using one element the first time around   In this scenario you could also exploit the powerful linq extension methods even more   var firstObject   objects First    return DoSomeThing firstObject  Concat DoSomeThingElse objects  ToList      It is possible to only evaluate the IEnumerable once in this case with some hassle  but profile first and see if it s really a problem

User · Answer

If you only need to check the first element you can peek on it without iterating the whole collection   public List lt object gt  Foo IEnumerable lt object gt  objects        object firstObject      if  objects    null     TryPeek ref objects  out firstObject           throw new ArgumentException         var list   DoSomeThing firstObject       var secondList   DoSomeThingElse objects       list AddRange secondList        return list     public static bool TryPeek lt T gt  ref IEnumerable lt T gt  source  out T first        if  source    null          throw new ArgumentNullException nameof source         IEnumerator lt T gt  enumerator   source GetEnumerator        if   enumerator MoveNext                  first   default T           source   Enumerable Empty lt T gt             return false             first   enumerator Current      T firstElement   first      source   Iterate        return true       IEnumerable lt T gt  Iterate                 yield return firstElement          using  enumerator                        while  enumerator MoveNext                                  yield return enumerator Current

User · Answer

Using IReadOnlyCollection lt T gt  or IReadOnlyList lt T gt  in the method signature instead of IEnumerable lt T gt   has the advantage of making explicit that you might need to check the count before iterating  or to iterate multiple times for some other reason   However they have a huge downside that will cause problems if you try to refactor your code to use interfaces  for instance to make it more testable and friendly to dynamic proxying  The key point is that IList lt T gt  does not inherit from IReadOnlyList lt T gt   and similarly for other collections and their respective read-only interfaces   In short  this is because  NET 4 5 wanted to keep ABI compatibility with earlier versions  But they didn t even take the opportunity to change that in  NET core    This means that if you get an IList lt T gt  from some part of the program and want to pass it to another part that expects an IReadOnlyList lt T gt   you can t  You can however pass an IList lt T gt  as an IEnumerable lt T gt    In the end  IEnumerable lt T gt  is the only read-only interface supported by all  NET collections including all collection interfaces  Any other alternative will come back to bite you as you realize that you locked yourself out from some architecture choices  So I think it s the proper type to use in function signatures to express that you just want a read-only collection    Note that you can always write a IReadOnlyList lt T gt  ToReadOnly lt T gt  this IList lt T gt  list  extension method that simple casts if the underlying type supports both interfaces  but you have to add it manually everywhere when refactoring  where as IEnumerable lt T gt  is always compatible    As always this is not an absolute  if you re writing database-heavy code where accidental multiple enumeration would be a disaster  you might prefer a different trade-off

User · Answer

If the aim is really to prevent multiple enumerations than the answer by Marc Gravell is the one to read  but maintaining the same semantics you could simple remove the redundant Any and First calls and go with   public List lt object gt  Foo IEnumerable lt object gt  objects        if  objects    null          throw new ArgumentNullException  objects         var first   objects FirstOrDefault         if  first    null          throw new ArgumentException               Empty enumerable not supported                  objects         var list   DoSomeThing first          var secondList   DoSomeThingElse objects        list AddRange secondList        return list      Note  that this assumes that you IEnumerable is not generic or at least is constrained to be a reference type

User · Answer

If your data is always going to be repeatable  perhaps don t worry about it  However  you can unroll it too - this is especially useful if the incoming data could be large  for example  reading from disk network    if objects    null  throw new ArgumentException    using var iter   objects GetEnumerator          if  iter MoveNext    throw new ArgumentException         var firstObject   iter Current      var list   DoSomeThing firstObject          while iter MoveNext              list Add DoSomeThingElse iter Current              return list      Note I changed the semantic of DoSomethingElse a bit  but this is mainly to show unrolled usage  You could re-wrap the iterator  for example  You could make it an iterator block too  which could be nice  then there is no list - and you would yield return the items as you get them  rather than add to a list to be returned

[c#] Handling warning for possible multiple enumeration of IEnumerable

Examples related to c#

Examples related to .net

Examples related to performance

Examples related to resharper