Closures in C# event handler delegates? [duplicate]

Question

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

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 2

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 = $"I am button number {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($"I am button number {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.