"Possible multiple enumeration of IEnumerable" vs "Parameter can be declared with base type"

Daniel Hilgarth picture Daniel Hilgarth · Jul 20, 2011 · Viewed 13.9k times · Source

In Resharper 5, the following code led to the warning "Parameter can be declared with base type" for list:

public void DoSomething(List<string> list)
{
    if (list.Any())
    {
        // ...
    }
    foreach (var item in list)
    {
        // ...
    }
}

In Resharper 6, this is not the case. However, if I change the method to the following, I still get that warning:

public void DoSomething(List<string> list)
{
    foreach (var item in list)
    {
        // ...
    }
}

The reason is, that in this version, list is only enumerated once, so changing it to IEnumerable<string> will not automatically introduce another warning. Now, if I change the first version manually to use an IEnumerable<string> instead of a List<string>, I will get that warning ("Possible multiple enumeration of IEnumerable") on both occurrences of list in the body of the method:

public void DoSomething(IEnumerable<string> list)
{
    if (list.Any()) // <- here
    {
        // ...
    }
    foreach (var item in list) // <- and here
    {
        // ...
    }
}

I understand, why, but I wonder, how to solve this warning, assuming, that the method really only needs an IEnumerable<T> and not a List<T>, because I just want to enumerate the items and I don't want to change the list.
Adding a list = list.ToList(); at the beginning of the method makes the warning go away:

public void DoSomething(IEnumerable<string> list)
{
    list = list.ToList();
    if (list.Any())
    {
        // ...
    }
    foreach (var item in list)
    {
        // ...
    }
}

I understand, why that makes the warning go away, but it looks a bit like a hack to me...
Any suggestions, how to solve that warning better and still use the most general type possible in the method signature?
The following problems should all be solved for a good solution:

  1. No call to ToList() inside the method, because it has a performance impact
  2. No usage of ICollection<T> or even more specialized interfaces/classes, because they change the semantics of the method as seen from the caller.
  3. No multiple iterations over an IEnumerable<T> and thus risking accessing a database multiple times or similar.

Note: I am aware that this is not a Resharper issue, and thus, I don't want to suppress this warning, but fix the underlying cause as the warning is legit.

UPDATE: Please don't care about Any and the foreach. I don't need help in merging those statements to have only one enumeration of the enumerable.
It could really be anything in this method that enumerates the enumerable multiple times!

Answer

SLaks picture SLaks · Jul 20, 2011

You should probably take an IEnumerable<T> and ignore the "multiple iterations" warning.

This message is warning you that if you pass a lazy enumerable (such as an iterator or a costly LINQ query) to your method, parts of the iterator will execute twice.