Problem with assigning delegates in for-loop

Question

Hi.
I have an application that is capable of plugins (MEF). The plugins are WPF UserControls that import services.

The user can select the wanted plugin from the main menu of the application.

To do this, i use the following loop:

foreach(IToolPlugin Plugin in ToolPlugins)
{
    Plugin.Init();
    MenuItem PluginMenuItem = Plugin.MenuItem; //New MenuItem but with Header set.
    PluginMenuItem.Click += new RoutedEventHandler(delegate(object o, RoutedEventArgs e) { DoSomething(Plugin.Control);});
    PluginsMenu.Items.add(PluginMenuItem);
}

That works very fine for a single item. But as soon as i have more than 1 plugin, all menuitems execute the delegate of the last loop. Or at least with the Plugin.Control of the last loop.

How can i fix this?
Thanks for any help.

Answer 1

On each iteration of the loop, you have to "capture" the value of the iterated value before you use it in a closure. Otherwise Plugin in each delegate will point to the last value of Plugin instead of the value that it held when the anonymous function was created.

You can read a more in depth explanation from Eric Lippert here:

Closing over the loop variable considered harmful - Fabulous Adventures in Coding

In short, the correct way to write your foreach loop is:

foreach(IToolPlugin Plugin in ToolPlugins)
{
    Plugin.Init();
    MenuItem PluginMenuItem = Plugin.MenuItem;

    IToolPlugin capturedPlugin = Plugin;

    PluginMenuItem.Click += 
        new RoutedEventHandler(delegate(object o, RoutedEventArgs e) {
            DoSomething(capturedPlugin.Control);
        });

    PluginsMenu.Items.add(PluginMenuItem);
}