Just recently, I got caught out by a closure. A closure is a piece of code that can be ‘executed later’, while still retaining the environment in which it was created. A lot of developers mistakenly think anonymous delegates, like lambda expressions, are by definition closures. They can be, but don’t have to (and usually aren’t).
I’ll illustrate with the actual code snippet that caught me out. Unfortunately, at my current client, I can’t make use of the excellent Visual Studio add-in ReSharper, which would have alerted me on this with a “Access to modified closure” warning.
foreach (string keyword in keywords)
{
ToolStripMenuItem keywordTsmi = new ToolStripMenuItem(text: keyword, image: null,
onClick: (sender, e) => Clipboard.SetText(keyword));
keywordsTsmi.DropDownItems.Add(keywordTsmi);
}
In this bit, I am filling up a context menu with new sub menu items, containing keywords that are copied to the clipboard when clicked on.
Obviously, this compiles just fine, and the submenu is correctly filled with all the distinct keywords. However, no matter which keyword you click, it would always copy the last keyword in the list. With this functionality being not-very-much-in-your-face, it slipped through (user) testing. I’ll be the first to admit we need more unit tests 😉
The solution is dead-simple:
foreach (string keyword in keywords)
{
var keyw = keyword;
ToolStripMenuItem keywordTsmi = new ToolStripMenuItem(text: keyw, image: null,
onClick: (sender, e) => Clipboard.SetText(keyw));
keywordsTsmi.DropDownItems.Add(keywordTsmi);
}
By assigning the value to a local-scope variable, the compiler will save a separate reference for each keyword, instead of only one that gets modified with each loop (hence the ReSharper warning). Because the latter is simply the standard behaviour when you use a delegate with a variable like that.
Room of a pleinolijf 