Closures in C# event handler delegates? [duplicate

Question

This question already has an answer here:

• Captured variable in a loop in C# 8 answers

I am coming from a functional-programming background at the moment, so forgive me if I do not understand closures in C#.

I have the following code to dynamically generate Buttons that get anonymous event handlers:

for (int i = 0; i < 7; i++)
{
    Button newButton = new Button();

    newButton.Text = "Click me!";

    newButton.Click += delegate(Object sender, EventArgs e)
    {
        MessageBox.Show("I am button number " + i);
    };

    this.Controls.Add(newButton);
}

I expected the text "I am button number " + i to be closed with the value of i at that iteration of the for loop. However, when I actually run the program, every Button says I am button number 7. What am I missing? I am using VS2005.

Edit: So I guess my next question is, how do I capture the value?

Answer 1

The closure captures the variable not the value. This means that by the time the delegate is executed, ie sometime after the end of the loop, the value of i is 6.

To capture a value, assign it to a variable declared in the loop body. On each iteration of the loop, a new instance will be created for each variable declared within it.

Jon Skeet's articles on closures has a deeper explanation and more examples.

for (int i = 0; i < 7; i++)
{
    var copy = i;

    Button newButton = new Button();

    newButton.Text = "Click me!";

    newButton.Click += delegate(Object sender, EventArgs e)
    {
        MessageBox.Show("I am button number " + copy);
    };

    this.Controls.Add(newButton);
}

Answer 2

You have created seven delegates, but each delegate holds a reference to the same instance of i.

The MessageBox.Show function is only called when the button is clicked. By the time the button has clicked, the loop has completed. So, at this point i will be equaling seven.

Try this:

for (int i = 0; i < 7; i++) 
{ 

    Button newButton = new Button(); 

    newButton.Text = "Click me!"; 

    int iCopy = i; // There will be a new instance of this created each iteration
    newButton.Click += delegate(Object sender, EventArgs e) 
    { 
        MessageBox.Show("I am button number " + iCopy); 
    }; 

    this.Controls.Add(newButton); 
}

Answer 3

By the time you click any button, they have all been generated from 1 thru 7, so they will all express the final state of i which is 7.

Answer 4

To get this behavior, you need to copy the variable locally, not use the iterator:

for (int i = 0; i < 7; i++)
{
    var inneri = i;
    Button newButton = new Button();
    newButton.Text = "Click me!";
    newButton.Click += delegate(Object sender, EventArgs e)
    {
        MessageBox.Show("I am button number " + inneri);
    };
    this.Controls.Add(newButton);
}

The reasoning is discussed in much greater detail in this question.

Answer 5

Nick has it right, but I wanted to explain a little better in the text of this question exactly why.

The problem isn't the closure; it's the for-loop. The loop only creates one variable "i" for the entire loop. It does not create a new variable "i" for each iteration. ^{Note: This has reportedly changed for C# 5.}

This means when your anonymous delegate captures or closes over that "i" variable it's closing over one variable that is shared by all the buttons. By the time you actually get to click any of those buttons the loop has already finished incrementing that variable up to 7.

The one thing I might do differently from Nick's code is use a string for the inner variable and build all those strings up front rather than at button-press time, like so:

for (int i = 0; i < 7; i++)
{
    var message = string.Format("I am button number {0}.", i);

    Button newButton = new Button();
    newButton.Text = "Click me!";
    newButton.Click += delegate(Object sender, EventArgs e)
    {
        MessageBox.Show(message);
    };
    this.Controls.Add(newButton);
}

That just trades a little bit of memory (holding on to larger string variables instead of integers) for a little bit of cpu time later on... it depends on your application what matters more.

Another option is to not manually code the loop at all:

this.Controls.AddRange(Enumerable.Range(0,7).Select(i => 
{ 
    var b = new Button() {Text = "Click me!", Top = i * 20};
    b.Click += (s,e) => MessageBox.Show(string.Format("I am button number {0}.", i));
    return b;
}).ToArray());

I like this last option not so much because it removes the loop but because it starts you thinking in terms of building this controls from a data source.