Don't Fall into the IEnumerable<T> Trap

Recently I upgraded some code of our company-internal class library and observed a plausible but still tricky problem. See yourself.

In a class deep inside the class library ( ;)) there was a method which looked similar to the following:

public IEnumerable<SomeObject> GetAllValidations()
{
   foreach...
       yield return new ValidationObject(...);
}

Then, inside another method in some other class there was the following code:

public void HandleValidationErrors(IEnumerable<ValidationObject> validationObjects)
{
    IDictionary<ValidationObject, IList<ValidationErrors>> cache = new Dictionary<ValidationObject, IList<ValidationErrors>>();

    foreach(var validationObj in validationObjects)
    {
        cache.Add(validationObj, validationObj.ValidationErrors.ToList());
    }

    ActivateValidatorsAccordingToValidationErrors(validationObjects, cache);

    //snip snip: If cache not empty, some validation objects haven't been treated/found
}


public void ActivateValidatorsAccordingToValidationErrors(IEnumerable<ValidationObject> validationObjects, IDictionary<ValidationObject, IList<ValidationErrors>> cache)
{
    foreach(var valObj in validationObjects)
    {
        var validator = GetValidatorByValidationObj(valObj);
        if(validator != null)
        {
            //activate validator
            //remove ValidationObj from cache
        }
    }
}

Now guess what, line 24 gave me an exception when I wanted to access the dictionary with the validationObj as key, telling me that the key was not present. I debugged the code and the obj was present!? My first thought: did someone override the Equals()??...but that wasn't the case.

Then I immediately saw the problem when looking at the method signature. Did you get it? IEnumerable<T>. Did you hear about the "lazy-evaluation" of IEnumerables? I did, fortunately. The problem here was that when the code entered the HandleValidationErrors(...) method, passing in the IEnumerable, that latter has not been evaluated so far. And then I had the situation of a possible multiple enumeration (Resharper tells you that, so don't ignore it!). So what does that mean? When the IEnumerable was accessed 1st in line 5, the method GetAllValidations() was called, actually executing the enumeration with new instances of type ValidationObject. So far so good. Then when the loop in line 18 was executed, the same happened, returning another, new instance of a ValidationObject, basically another IEnumerable<ValidationObject> than I previously added to the cache variable. And here we are, now the exception when accessing the dictionary is quite obvious, isn't it?

Obviously this was a silly mistake and fixing it was quite easy by inserting somewhere (I did it in the first possible place to keep other devs from falling in this trap as well ;)) a line like

var theList = theValidationObjectEnumeration.ToList<ValidationObject>()

which "materializes" the list and prevents the multiple enumeration problem.

I'm writing this post because such bugs can become quite nasty to find, especially if you don't know the fact about lazy-loading of IEnumerable<T> and if the method returning the IEnumerable was written by some other dev. So watch out!

---

Questions? Thoughts? Hit me up on Twitter