Action delegate uses the last values of variables declared outside foreach loop

Question

I have this piece of code:

int i = 0;
foreach(var tile in lib.dic.Values)
{
    var ii = i;
    var t = tile;
    Button b = new Button( () = > { MainStatic.tile = t; } );
    Checkbox c = new Checkbox( () = > { lib.arr[ii].b = !lib.arr[ii].b; } );
    i++;
}

While the above code works as it should, this piece below:

int i = 0;
foreach(var tile in lib.dic.Values)
{
    Button b = new Button( () = > { MainStatic.tile = tile; } );
    Checkbox c = new Checkbox( () = > { lib.arr[i].b = !lib.arr[i].b; } );
    i++;
}

…will always execute the delegates with the last values of i and tile variables. Why does this happen, and why do I have to make a local copy of those vars, especially non-reference type int i?

This is an extremely frequently asked question. See http://stackoverflow.com/questions/8898925/is-there-a-reason-for-cs-reuse-of-the-variable-in-a-foreach/8899347#8899347 — Eric Lippert, Jan 06 '13 at 17:57

MBen · Accepted Answer · 2013-01-06T17:15:13.520

3

Known "issue", please check Eric's blog Closures, captured variables.

Microsof decided to go for a breaking change, and fix it in C# 5.

edited Jan 06 '13 at 17:15

answered Jan 06 '13 at 17:09

MBen

3,956
21
25

1

I wouldn't describe it as a "known issue" - it is by design. – Colin Mackay Jan 06 '13 at 17:12
@ColinMackay Indeed. I knew a I will have a comment like this, however they seem to have fixed it in C# 5. – MBen Jan 06 '13 at 17:13
1

Interesting, I didn't know they'd changed it in C# 5. Perhaps too many people didn't understand what was going on. I can't think why I'd want the old behaviour but I'm sure someone will be relying on it now. – Colin Mackay Jan 06 '13 at 17:16

Sergey Kalinichenko · Answer 2 · 2013-01-06T17:16:52.133

This is expected: when you make a lambda, compiler creates a closure. It will capture the value of a temporary variable in there, but it would not capture the value of loop variables and other variables that change after creation of the lambda.

The core of the issue is that the delegate creation and execution times are different. The delegate object is created while the loop is running, but it is called well after the loop has completed. At the time the delegate is called, the loop variable has the value that it reached at the time the loop has completed, resulting in the effect that you see (the value does not change, and you see the last value from the loop).

Forgetting to create a temporary variable for use in closures is a mistake so common that popular code analyzers (e.g. ReSharper) warn you about it.

And this is how I find out about ReSharper :) – user1306322 Jan 06 '13 at 17:19 — user1306322, Jan 06 '13 at 17:19

score 2 · Answer 3 · answered Jan 06 '13 at 17:11

You cannot use loop variables like this because by the time the delegate is executed the loop variable will likely be in its final (end of loop) state as it uses the value of the variable at the time the delete is executed, not created.

You need to make a local copy of the variable to get this to work:

int i = 0;
foreach(var tile in lib.dic.Values)
{
    var tileForClosure = tile;
    var iForClosure = i;
    Button b = new Button( () = > { MainStatic.tile = tileForClosure ; } );
    Checkbox c = new Checkbox( () = > { lib.arr[iForClosure].b = !lib.arr[iForClosure].b; } );
    i++;
}

By creating a local copy on each loop the value does not change and so your delegate will use the value that you expect.

Action delegate uses the last values of variables declared outside foreach loop

3 Answers3

Linked

Related