6


Lately I've read C# in Depth and it taught me about lambda expression, I've been using them to pas data to click events and such like:

image.MouseDown+=(o ,e)=>MethodToDoSomething(DataNeededForAction);

now the problem with those is variable capturing when used in a foreach loop (thank you Jon Skeet for making that part really clear :), when initializing several objects that have events that I subscribe to I usually bump into an issue with variable capturing. consider the following example:

foreach (var game in GamesCollection)
{
    Image img = new Image();
    img.Width = 100;
    img.MouseDown+=(o,e) => MyMethod(game.id);
}

To avoid capturing in this situation I have to add some variable to assign game to and then pass that variable to the method, this creates extra unclear code and mostly, additional clutter. Is there a way to bypass this? Something that'll at least look cleaner?

Thx, Ziv

Ziv
  • 2,755
  • 5
  • 30
  • 42

4 Answers4

6

(EDIT: Note that this has changed with C# 5, where foreach now effectively creates a fresh variable for each iteration.)

No, there's no way of avoiding this. Basically it was a mistake in the way the language specification described foreach in terms of a single variable, rather than a new variable per iteration. Personally I don't see it as a problem in terms of the amount of code involved - by the time you've realised that it's a problem and figured out how to fix it, you've got over the biggest hurdle. I'd normally include a comment to make it obvious to maintenance programmers, mind you.

I'm sure the C# team would do it differently if they were starting from scratch, but they're not. Heck, they've even discussed changing the existing behaviour... but there are good reasons against that change (notably that code which works correctly on a C# N+1 compiler would still compile, but give the wrong result on a C# N compiler; a very subtle source of bugs for open source library authors who expect their code to be built with multiple compilers).

(Hope you're enjoying the book, btw...)

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • Since this answer was written, the C# team has decided to make the breaking change of moving the loop variable logically inside the loop in C# 5 (the linked article is updated). The code in the question now works without having to copy the `game` variable. – Anders Abel Nov 06 '12 at 19:39
  • @AndersAbel: Indeed. Unfortunately there are far too many answers to go back and fix them all now :( – Jon Skeet Nov 06 '12 at 19:41
2

In short, no. You need to create extra storage to keep the correct instance of game available in the method closure.

spender
  • 117,338
  • 33
  • 229
  • 351
  • I understand this is a limitation of the compiler, but I wonder if it would be possible to improve the compiler to detect it and automate it. Is there any technical reason that this wouldn't be possible? – Mark H Apr 06 '11 at 23:11
  • It's simply not how it works. foreach only uses a single variable declaration for storing the Enumerator.Current which gets overwritten on each pass. This is what gets captured in the method closure. I don't see that it needs changing. – spender Apr 06 '11 at 23:14
  • I know that's not how it works, but I'm asking why make the decision to do it that way. A compiler could potentially detect that the iteration variable is captured and account for it by adding the temporary assignment automatically. Almost every problem I see with people using closures is due to capturing the iteration variable. I'm asking if there's any technical/logical reason as to why a future language/compiler could not automate it, as I can't think of any practical reason where anybody would want to capture the iteration variable. – Mark H Apr 06 '11 at 23:19
  • 1
    I think that making a special case would prove more confusing than beneficial and effectively require an understanding of two different modes of operation. – spender Apr 06 '11 at 23:28
0

If GamesCollection is a List<T>, you can use the ForEach method:

GamesCollection.ForEach(game => {
    Image img = new Image();
    img.Width = 100;
    img.MouseDown+=(o,e) => MyMethod(game.id);
});

But in general, no. And one extra variable is not really that cluttered.

Gabe Moothart
  • 31,211
  • 14
  • 77
  • 99
  • Sure, but effectively this creates a new reference to each Game in GamesCollection in the parameter of the delegate that you supply to ForEach, so it's really much the same thing. – spender Apr 06 '11 at 22:48
  • @spender right. As I read the question, he was trying to avoid extra lines of code, not necessarily extra references. – Gabe Moothart Apr 06 '11 at 22:54
-1

What about pulling out the game.id first with a LINQ expression. Pushing the call through .Aggregate() should make a new variable each time because it is really a method parameter.

GamesCollection.Select(game => game.id).Aggregate((_, id) => {
    Image img = new Image();
    img.Width = 100;
    img.MouseDown+=(o,e) => MyMethod(id);
    return img;
});
Jason
  • 3,736
  • 5
  • 33
  • 40