C# Closure is not working as expected

Question

I can't understand quite clearly the difference between two blocks of code. Consider there is a program

    class Program
{
    static void Main(string[] args)
    {

        List<Number> numbers = new List<Number>
                                   {
                                       new Number(1),
                                       new Number(2),
                                       new Number(3)
                                   };


        List<Action> actions = new List<Action>();
        foreach (Number numb in numbers)
        {
            actions.Add(() => WriteNumber(numb));
        }

        Number number = null;
        IEnumerator<Number> enumerator = numbers.GetEnumerator();
        while (enumerator.MoveNext())
        {
            number = enumerator.Current;
            actions.Add(() => WriteNumber(number));
        }

        foreach (Action action in actions)
        {
            action();
        }

        Console.ReadKey();


    }

    public static void WriteNumber(Number num)
    {
        Console.WriteLine(num.Value);
    }

    public class Number
    {
        public int Value;

        public Number(int i)
        {
            this.Value = i;
        }

    }

}

The output is

These two blocks of code should work identically. But you can see that the closure is not working for the first loop. What am i missing?

Thanks in advance.

Is this the broken `foreach` closure issue? (http://stackoverflow.com/questions/512166/c-sharp-the-foreach-identifier-and-closures) — sircodesalot, Oct 03 '13 at 14:23
@sircodesalot: Sort of the opposite - this is `foreach` behaving sensibly due to C# 5 fixing the issue. — Jon Skeet, Oct 03 '13 at 14:27
They fixed in in C# 5.0 Though I used version 3.5 and it is surprisingly working — user2843021, Oct 07 '13 at 06:04

score 3 · Answer 1 · answered Oct 03 '13 at 14:26

You declare the number variable outside of your while loop. For each number you store the reference of it in your number variable - every time overwriting the last value.

You should just move the declaration inside the while-loop, so you have a new variable for each of your numbers.

    IEnumerator<Number> enumerator = numbers.GetEnumerator();
    while (enumerator.MoveNext())
    {
        Number number = enumerator.Current;
        actions.Add(() => WriteNumber(number));
    }

score 3 · Answer 2 · answered Oct 03 '13 at 14:26

These two blocks of code should work identically.

No they shouldn't - at least in C# 5. In C# 3 and 4 they would, in fact.

But in the foreach loop, in C# 5, you have one variable per iteration of the loop. Your lambda expression captures that variable. Subsequent iterations of the loop create different variables which don't affect the previously-captured variable.

In the while loop, you have one variable which all the iterations capture. Changes to that variable will be seen in all of the delegates that captured it. You can see this by adding this line after your while loop:

number = new Number(999);

Then your output would be

Now in C# 3 and 4, the foreach specification was basically broken by design - it would capture a single variable across all iterations. This was then fixed in C# 5 to use a separate variable per iteration, which is basically what you always want with that sort of code.

score 1 · Answer 3 · answered Oct 03 '13 at 14:28

In your loop:

    Number number = null;
    IEnumerator<Number> enumerator = numbers.GetEnumerator();
    while (enumerator.MoveNext())
    {
        number = enumerator.Current;
        actions.Add(() => WriteNumber(number));
    }

number is declared outside of the loop scope. So when it gets set to the next current iterator, all your action refernces to number also get updated to the latest. So when you run each action, they will all use the last number.

score 0 · Answer 4 · answered Oct 04 '13 at 09:43

0

Thanks for all your answers. But I think I was misunderstood. I WANT the closue to work. That's why i set the loop variable out of scope. The question is: Why does not it work in the first case? I forgot to mention that I use C# 3.5 (not C# 5.0). So the soop variable should be defined out of scope and two code blocks shoul work identically.

answered Oct 04 '13 at 09:43

user2843021

39
1

You really should add this to your question. This isn't really an answer. – juharr Oct 04 '13 at 11:33

C# Closure is not working as expected

4 Answers4