Problem with different "execution context" of an anonymous method within a loop

Question

I have a problem with an anonymous method within a loop.

The following code is just to illustrate my problem:

private void Form1_Load(object sender, EventArgs e)
{
    List<string> bassists = new List<string>(){
        "Jaco Pastorius", 
        "Marcus Miller", 
        "Flea", 
        "Vicor Wooten"
    };

    foreach (string item in bassists)
    {
        this.button1.Click += (s, ea) => Output(s, ea, item); 
    }
}

private void Output(object s, EventArgs e, string item)
{
    this.listBox1.Items.Add(item);
}

And when I click into the button, the output is:

Victor Wooten
Victor Wooten
Victor Wooten
Victor Wooten

instead of:

Jaco Pastorius
Marcus Miller
Flea
Vicor Wooten

The main point of my problem is the differents execution context. I know my example is stupid.

Jaco Pastorius, Marcus Miller, Flea and Victor Wooten. One of these is not like the others.... — jason, Dec 10 '10 at 14:08

score 12 · Accepted Answer · answered Dec 10 '10 at 14:01

12

This is the captured variable problem. Fix it by changing

foreach (string item in bassists)
{
    this.button1.Click += (s, ea) => Output(s, ea, item); 
}

to

foreach (string item in bassists)
{
    string currentItem = item;
    this.button1.Click += (s, ea) => Output(s, ea, currentItem); 
}

Here is an explanation of the issue: Closing over loop variable considered harmful. By putting the local variable currentItem in the scope of the loop and closing over that, we now capture that variable instead of the loop variable.

answered Dec 10 '10 at 14:01

jason

236,483
35
423
525

You can also use an indexed loop – Falcon Dec 10 '10 at 14:04
@Falcon: What would be the advantage of using an indexed loop? – Cody Gray - on strike Dec 10 '10 at 14:05
@Cody: I guess Falcon meant that using a `for(int i...)` the string would be necessary inside the scope ;-) – digEmAll Dec 10 '10 at 14:06

Aliostad · Answer 2 · 2010-12-10T14:07:27.523

0

Your problem is that you are creating new handlers in the loop, that is unnecessary and dangerous.

Also you are creating an anonymous method which has the value in the loop hard-coded. That is worse.

edited Dec 10 '10 at 14:07

answered Dec 10 '10 at 14:01

Aliostad

80,612
21
160
208

score 0 · Answer 3 · answered Dec 10 '10 at 14:12

0

In whatever cases Jason's answer is correct. It is problem of variable capturing. This will majorly occur in two situation Threading and Anonymous methods

answered Dec 10 '10 at 14:12

manu

1,807
4
25
32

Problem with different "execution context" of an anonymous method within a loop

3 Answers3