views:

428

answers:

3

My problem is hard to explain, so I created an example to show here.

When the WPF window in the example below is shown, three buttons are displayed, each one with a different text.

When anyone of these buttons is clicked, I assume its text should be displayed in the message, but instead, all of them display the same message, as if all of them were using the event handler of the last button.


    public partial class Window1 : Window {
        public Window1() {
            InitializeComponent();
            var stackPanel = new StackPanel();
            this.Content = stackPanel;
            var n = new KeyValuePair[] { 
                new KeyValuePair("I", () => MessageBox.Show("I")), 
                new KeyValuePair("II", () => MessageBox.Show("II")), 
                new KeyValuePair("III", () => MessageBox.Show("III"))
            };
            foreach (var a in n) {
                Button b = new Button();
                b.Content = a.Key;
                b.Click += (x, y) => a.Value();
                stackPanel.Children.Add(b);
            }
        }
    }

Does anyone know what is wrong?

A: 

To fix this do the following:

var value = a.Value();
b.Click += (x, y) => value;
alex
+2  A: 

It is because of how closures are evaluated compiler in the loop:

foreach (var a in n) {
    Button b = new Button();
    b.Content = a.Key;
    b.Click += (x, y) => a.Value();
    stackPanel.Children.Add(b);
}

The compiler assumes that you will need the context of a in the closure, since you are using a.Value, so it is creating one lambda expression that uses the value of a. However, a has scope across the entire loop, so it will simply have the last value assigned to it.

To get around this, you need to copy a to a variable inside the loop and then use that:

foreach (var a in n) {
    Button b = new Button();
    b.Content = a.Key;

    // Assign a to another reference.
    var a2 = a;

    // Set click handler with new reference.
    b.Click += (x, y) => a2.Value();
    stackPanel.Children.Add(b);
}
casperOne
+1  A: 

Unintuitive, huh? The reason is because of how lambda expressions capture variables from outside the expression.

When the for loop runs for the last time, the variable a is set to the last element in the array n, and it never gets touched after that. However, the lambda expression attached to the event handler, which returns a.Value(), hasn't actually been evaluated yet. As a result, when it does run, it'll then get the current value of a, which by then is the last element.

The easiest way to make this work the way you expect it to without using a bunch of extra variables and so forth would probably to change to something like this:

var buttons = n.Select(a => {
    Button freshButton = new Button { Content = a.Key };
    freshButton.Click += (x, y) => a.Value();
    return freshButton;
});

foreach(var button in buttons)
    stackPanel.Children.Add(button);
mquander