Tripping on Code

One of the many warnings that Resharper throws at us is Code Inspection: Possible multiple enumeration of IEnumerable. When I was younger and much less experienced, this was one of those warnings that I never fully understood. Many of my methods returned IEnumerables rather than any of it's derivations. I started invoking ToList every time resharper complained, without completely understanding the reason for the warning. I understood that an IEnumerable could be lazily evaluated and that it could hurt performance to evaluate it again and again. I did not realize until recently that because of lazy evaluation, we could get vastly different results.

Consider the code below:

using System;
using System.Linq;
using System.Collections.Generic;

public class Person
{
	public int Id { get; set; }

	public string Name { get; set; }

    public int Age { get; set; }
}

public class PersonGenerator
{
	private Random random = new Random();
	public IEnumerable<Person> GetRandomPeople(int maxCount)
	{
		var count = this.random.Next(maxCount);
		for (var i = 0; i < count; i++)
		{
			yield return new Person
            {
                Id = random.Next(int.MaxValue),
                Age = random.Next(86),
            };
		}
	}
}

public class Program
{
	public static void Main()
	{
		PersonGenerator personGenerator = new PersonGenerator();
		var randomPeople = personGenerator.GetRandomPeople(100);
		var count1 = randomPeople.Count();
		var count2 = randomPeople.Count();

		Console.WriteLine($"Count1 = {count1}, Count2 = {count2}");
	}
}

The code above outputs the following

Count1 = 4, Count2 = 34

Wait. What's happening here?

The GetRandomPeople method uses yield return. This causes it to return a type which is lazily evaluated. Every time the Count() method is invoked, The GetRandomPeople method is invoked again. Since that method returns a random number of items, the counts are different for every enumeration of the IEnumerable.

The lazy evaluation of the IEnumerable is a very powerful feature, especially paired with a yield return, however to quote a tired old adage, "With great power comes great responsibility".

How do I prevent this?

• The first thing that should have been done is to realize that multiple enumeration was occuring and invoke ToList on it.
• There is no reason for the GetRandomPeople method to either return an IEnumerable or use a yield return. The method should return a List, a ReadOnlyCollection an ImmutableList or any other concrete collection implementation.

Microsoft has published a set of Guidelines for Collections that is a must read for all developers.