Quirks Of Using For Each

Collections can be tricky sometimes when you’re writing code. This is especially true when you need to modify them while you’re looping through them. Let’s look at a couple examples that can get you into trouble.

//create a typical list of items
List<string> items = new List<string> {
    "first", "second", "third", "fourth"
};

//now clear the list
foreach (string item in items) {
    items.Remove(item);
}

Of course, this won’t work. Why?

InvalidOperationException: Collection was modified; enumeration operation may not execute

Once our loop has started the boundaries of the collection have been defined. Adding or removing items will change those boundaries and create errors. Naturally, most languages aren’t going to like this and will crash.

Fortunately, we do get an Exception message (which is the best kind of error message). However, we should be more concerned about subtle problems that could arise if we change a collection while looping through it. For example, consider the ForEach method attached to Generic Lists…

//get a list of the employees that 
//should be deleted from the system
List<string> firedEmployees = new List<string> {
    "jeff", "blake", "steve"
};

//and delete them
firedEmployees.ForEach(employee => {
    EmployeeSystem.Delete(employee);
    firedEmployees.Remove(employee);
});

Everyone deleted? Nope…

I don’t know about you, but I was surprised to find that one employee remained on this list after the code was run (the second employee at that). ForEach made sure to keep track of our collection as it changed but unfortunately it created an undesirable result.

To take it a step further, if you were to add a new item each time then you’d end up with an infinite loop, an error that would be caught sooner but a problem all the same.

This straight forward code has potential for some serious problems.

So, abandon the framework! Move to Java! .NET is doomed!

…Or we can just write an extension method to take care of both problems at the same time.

public static class EnumerableExtensions {

    /// <summary>
    /// Safely enumerates through a collection
    /// </summary>
    public static IEnumerable<T> Each<T>(this IEnumerable<T> collection, Action<T> action) {

        //move the collection into its own list and
        //perform the work on each item
        collection.ToList().ForEach(action);
        return collection;

    }

}

The fix itself isn’t very impressive. One additional call to ToList() and we’re pretty much okay. But this does keep the original collection safe while we enumerate through the protected one.

Slightly Easier Recursive Functions

I’ve joked before that recursive functions were the kind that make you “curse over and over and over and over…”. Granted, they aren’t the most difficult thing to do in programming, they can tend to require a little more focus to write correctly.

So when you work with a recursive function you normally need to create some sort of safety counter to prevent an infinite loop. If not you end up passing around your counter by reference which can be a bit confusing to developers in itself.

Not only that, it also tends to hurt the “black box” approach to class design. Even if the method is private you still need to prepare a safety counter in advance before you can call the method — a function that has a order dependency, which isn’t a good thing.

Lambda To The Rescue

I love working with lambdas – I think that I cried a tear of joy the day I realized they could be passed as an argument (actually, I didn’t… that would be weird).

So what if we created a single lambda within our functions that would perform all of the recursive logic inline?

public void ScanElements(XElement xml) {
    
    //create a safety counter
    int safety = 0;
    
    //declare the scan action as null at first
    //since we can't call it recursively inside
    //the method unless it is declared first
    Action<XElement> scan = null;

    //assign the actual work to do
    scan = (node) => {

        //increment the safety
        if (safety++ >= 100) { return; }

        //do the actual work here
        Console.WriteLine(node.Name);

        //and then recursively call the scan method again
        foreach (XElement element in node.Elements()) {
            scan(element);
        }

        //reverse the safety
        safety--;

    };

    //now, actually start the recursive function here
    scan(xml);

}

Now, that isn’t too bad. We have all of our logic encapsulated into a single method that doesn’t require any setup before it is used. It might be a little odd to a developer that hasn’t used lambdas much, but a comment explaining what is going on should be enough to get them onto the right track.

Maybe this is a terrible idea – I’m not really sure but it does decrease complexity and improve encapsulation — even if it is with a single private method.

Using the .Descendants() method as part of the XElement/XDocument classes would be much wiser here – this code is just an example of performing a recursive method.