Gotcha: Don’t use anonymous delegates within loops

Okay, what do you think the following code prints?

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

class Program
{
    static void Main(string[] args)
    {
        var strings = new string[] { "Hello ", "World" };
        var actions = new List<Action>();
        foreach (var s in strings)
        {
            actions.Add(() => Console.Write(s));
        }
        actions.ForEach(action => action());
    }
}

If you think it says “WorldWorld”, you don’t really need to read the rest of this article.  You can skip to the section entitled “So you think you’re so smart”.

If, on the other hand, you think it prints “Hello World”, it’s worth explaining why the code doesn’t behave as expected.  The issue is the semantics of anonymous delegates.  Variables referenced within anonymous delegates are “live”.  If they change within the function, the latest value is used.  If they go out of scope, the last value is used.

The moral of this story is: don’t create a delegate in a loop, because it probably won’t do what you’re expecting.*  This bit me recently whilst trying to write code to stop a set of threads simultaneously.  The best option is to refactor the code and extract the line that adds in the action.  Of course, that gives you an ugly dialog box to deal with.

*Actually, any time the variable is changes before you use it, but loops are the most common case.

Actually, what the dialog is telling you is that extracting the method will result in the behaviour changing.  Luckily that’s what we actually want.  So, here you go, an even less elegant way to print “Hello World” on the console:

    static void Main(string[] args)
    {
        var strings = new string[] { "Hello ", "World" };
        var actions = new List<Action>();
        foreach (var s in strings)
        {
            AddAction(actions, s);
        }
        actions.ForEach(action => action());
    }

    private static void AddAction(List<Action> actions, string s)
    {
        actions.Add(() => Console.Write(s));
    }

So you think you’re so smart

Okay, how about this code: 

    static void Main(string[] args)
    {
        var actions = from s in new string[] { "Hello ", "World" }
                      select new Action(() => Console.Write(s));
        actions.ToList().ForEach(action => action());
    }

If anyone can explain why this prints “Hello World” succinctly, there’s a beer in it.  The interesting things is that the “ToList” ensures that all of the actions are produced prior to execution, so it’s not to do with the co-routing aspect of Linq.  I believe it’s an example of why expressions of delegates and delegates are slightly different.  A more prosaic explanation would be simply to point out that Linq would be horribly broken if this didn’t behave.