Resharper's 'Possible Multiple enumeration of IEnumerable warning'

Question

Not Duplicate: I do not feel this is a duplicate as in my specific case I feel it's actually better to ignore the warning.

For example,

IEnumerable<Item> milionItems = GetAMillionItemsFromDatabase();

var item1 = millionItems.FirstOrDefault(x=> x.Condition == "Excellent");

var item2 = millionItems.FirstOrDefault(x=> x.Condition == "Good");

I get the warning message under 'millionItems' and I know what it means however I'm not sure if it's always worth ToList'ing just to get rid of it.

GetAMillionItemsFromDatabase().ToList();

That seems awful performance wise as it would bring in a million records into memory at once.

However if I don't do that and proceed to enumerate the IEnumerable and even though it would hit the database twice, it wouldn't bring all the data in as it would find the first matching item and return. In this case it seems to me it's better to actually ignore the message.

Sure, it *can be* safe to ignore the warning. It depends on what's being enumerated. Some classes that implement `IEnumerable` will return an empty collection if you try to enumerate them twice. You need to determine what's best in your case: keeping all the items in a list, making multiple potentially expensive database calls, or maybe even foregoing LINQ and writing a loop that will get `item1` and `item2` in a single pass over the data. `Enumerable.Aggregate` *might* be useful in this case, as well. — Jim Mischel, Aug 11 '16 at 18:42

score 5 · Accepted Answer · answered Aug 10 '16 at 21:49

There is a very good chance that going to DB twice in this case would be better than client side search via IEnumerable which is performed in current code.

If you can't push search to DB (i.e. by keeping IQueryable<Item> to allow chaining) you still can somewhat optimize the lookup by checking for both conditions on each item:

  foreach(var x in millionItems)
  {
     item1 = item1 == null && x=> x.Condition == "Excellent" ? x : item1;
     item2 = item2 == null && x=> x.Condition == "Good" ? x : item2;

     if (item1 != null && item2 != null)
     {
           break;
     }
  }

This have good chance to go through a lot of items client side so, but at least it will not keep them in memory at the same time.

Converting to list with ToList is unlikely to be better if these are just 2 queries you need to build.

Resharper's 'Possible Multiple enumeration of IEnumerable warning'

1 Answers1