3

I can't understand quite clearly the difference between two blocks of code. Consider there is a program

 class Program { static void Main(string[] args) { List<Number> numbers = new List<Number> { new Number(1), new Number(2), new Number(3) }; List<Action> actions = new List<Action>(); foreach (Number numb in numbers) { actions.Add(() => WriteNumber(numb)); } Number number = null; IEnumerator<Number> enumerator = numbers.GetEnumerator(); while (enumerator.MoveNext()) { number = enumerator.Current; actions.Add(() => WriteNumber(number)); } foreach (Action action in actions) { action(); } Console.ReadKey(); } public static void WriteNumber(Number num) { Console.WriteLine(num.Value); } public class Number { public int Value; public Number(int i) { this.Value = i; } } } 

The output is

1 2 3 3 3 3 

These two blocks of code should work identically. But you can see that the closure is not working for the first loop. What am i missing?

Thanks in advance.

4
  • Is this the broken foreach closure issue? (stackoverflow.com/questions/512166/…) Commented Oct 3, 2013 at 14:23
  • 1
    @sircodesalot: Sort of the opposite - this is foreach behaving sensibly due to C# 5 fixing the issue. Commented Oct 3, 2013 at 14:27
  • Oh they fixed it. Good to know, thanks! Commented Oct 3, 2013 at 14:27
  • They fixed in in C# 5.0 Though I used version 3.5 and it is surprisingly working Commented Oct 7, 2013 at 6:04

4 Answers 4

3

You declare the number variable outside of your while loop. For each number you store the reference of it in your number variable - every time overwriting the last value.

You should just move the declaration inside the while-loop, so you have a new variable for each of your numbers.

 IEnumerator<Number> enumerator = numbers.GetEnumerator(); while (enumerator.MoveNext()) { Number number = enumerator.Current; actions.Add(() => WriteNumber(number)); } 
Sign up to request clarification or add additional context in comments.

Comments

3

These two blocks of code should work identically.

No they shouldn't - at least in C# 5. In C# 3 and 4 they would, in fact.

But in the foreach loop, in C# 5, you have one variable per iteration of the loop. Your lambda expression captures that variable. Subsequent iterations of the loop create different variables which don't affect the previously-captured variable.

In the while loop, you have one variable which all the iterations capture. Changes to that variable will be seen in all of the delegates that captured it. You can see this by adding this line after your while loop:

number = new Number(999); 

Then your output would be

1 2 3 999 999 999 

Now in C# 3 and 4, the foreach specification was basically broken by design - it would capture a single variable across all iterations. This was then fixed in C# 5 to use a separate variable per iteration, which is basically what you always want with that sort of code.

Comments

1

In your loop:

 Number number = null; IEnumerator<Number> enumerator = numbers.GetEnumerator(); while (enumerator.MoveNext()) { number = enumerator.Current; actions.Add(() => WriteNumber(number)); } 

number is declared outside of the loop scope. So when it gets set to the next current iterator, all your action refernces to number also get updated to the latest. So when you run each action, they will all use the last number.

Comments

0

Thanks for all your answers. But I think I was misunderstood. I WANT the closue to work. That's why i set the loop variable out of scope. The question is: Why does not it work in the first case? I forgot to mention that I use C# 3.5 (not C# 5.0). So the soop variable should be defined out of scope and two code blocks shoul work identically.

1 Comment

You really should add this to your question. This isn't really an answer.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.