Resharper warning on *all* uses of an enumerable about "Possible Multiple Enumeration"

Question

I'm getting the "Possible Multiple Enumeration of IEnumerable" warning from Reshaper. How to handle it is already asked in another SO question. My question is slightly more specific though, about the various places the warning will pop up.

What I'm wondering is whether or not Resharper is correct in giving me this warning. My main concern is that the warning occurs on all instances of the users variable below, indicated in code by "//Warn".

My code is gathering information to be displayed on a web page in a grid. I'm using server-side paging, since the entire data set can be tens or hundreds of thousands of rows long. I've commented the code as best as possible.

Again, please let me know whether or not this code is susceptible to multiple enumerations. My goal is to perform my filtering and sorting of data before calling ToList(). Is that the correct way to do this?

private List<UserRow> GetUserRows(UserFilter filter, int start, int limit,
                                  string sort, SortDirection dir, out int count)
{
    count = 0;

    // LINQ applies filter to Users object
    var users = (
            from u in _userManager.Users
            where filter.Check(u)
            select new UserRow
                        {
                            UserID = u.UserID,
                            FirstName = u.FirstName,
                            LastName = u.LastName,
                            // etc.
                        }
        );

    // LINQ orders by given sort
    if (!String.IsNullOrEmpty(sort))
    {
        if (sort == "UserID" && dir == SortDirection.ASC)
            users = users.OrderBy(u => u.UserID); //Warn
        else if (sort == "UserID" && dir == SortDirection.DESC)
            users = users.OrderByDescending(u => u.UserID); //Warn
        else if (sort == "FirstName" && dir == SortDirection.ASC)
            users = users.OrderBy(u => u.FirstName); //Warn
        else if (sort == "FirstName" && dir == SortDirection.DESC)
            users = users.OrderByDescending(u => u.FirstName); //Warn
        else if (sort == "LastName" && dir == SortDirection.ASC)
            users = users.OrderBy(u => u.LastName); //Warn
        else if (sort == "LastName" && dir == SortDirection.DESC)
            users = users.OrderByDescending(u => u.LastName); //Warn
        // etc.
    }
    else
    {
        users = users.Reverse(); //Warn
    }

    // Output variable
    count = users.Count(); //Warn

    // Guard case - shouldn't trigger
    if (limit == -1 || start == -1)
        return users.ToList(); //Warn

    // Pagination and ToList()
    return users.Skip((start / limit) * limit).Take(limit).ToList(); //Warn
}

Answer 1

Yes, ReSharper is right: count = users.Count(); enumerates unconditionally, and then if the limit or the start is not negative 1, the ToList would enumerate users again.

It appears that once ReSharper decides that something is at risk of being enumerated multiple times, it flags every single reference to the item in question with the multiple enumeration warning, even though it's not the code that is responsible for multiple enumeration. That's why you see the warning on so many lines.

A better approach would add a separate call to set the count. You can do it upfront in a separate statement, like this:

count = _userManager.Users.Count(u => filter.Check(u));

This way you would be able to leave users in its pre-enumerated state until the final call of ToList.

Answer 2

Your warning is hopefully generated by the call to Count, which does run an extra query.

In the case where limit == -1 || start == -1 you could make the ToList call and then get the count from that, but in the general case there's nothing you can do - you are making two queries, one for the full count and one for a subset of the items.

I'd be curious to see whether fixing the special case causes the warning to go away.

---

Edit: As this is LINQ-to-objects, you can replace the last return line with a foreach loop that goes through your whole collection counting them, but also builds up your restricted skip/take sublist dynamically, and thus only iterates once.

You could also benefit from only projecting (select new UserRow) in this foreach loop and just before your special-case ToList, rather than projecting your whole collection and then potentially discarding the majority of your objects.

var users = _userManager.Users.Where(u => filter.Check(u));

// Sort as above

List<UserRow> rtn;
if (limit == -1 || start == -1)
{
    rtn = users.Select(u => new UserRow { UserID = u.UserID, ... }).ToList();
    count = rtn.Length;
}
else
{
    int takeFrom = (start / limit) * limit;
    int forgetFrom = takeFrom + limit;
    count = 0;
    rtn = new List<UserRow>();
    foreach(var u in users)
    {
        if (count >= takeFrom && count < forgetFrom)
            rtn.Add(new UserRow { UserID = u.UserID, ... });
        count++;
    }
}
return rtn;