7

In the following program, DummyMethod always print 5. But if we use the commented code instead, we get different values (i.e. 1, 2, 3, 4). Can anybody please explain why this is happenning?

        delegate int Methodx(object obj);

        static int DummyMethod(int i)
        {
            Console.WriteLine("In DummyMethod method i = " + i);
            return i + 10;
        }

        static void Main(string[] args)
        {    
            List<Methodx> methods = new List<Methodx>();

            for (int i = 0; i < 5; ++i)
            {
                methods.Add(delegate(object obj) { return DummyMethod(i); });
            }

            //methods.Add(delegate(object obj) { return DummyMethod(1); });
            //methods.Add(delegate(object obj) { return DummyMethod(2); });
            //methods.Add(delegate(object obj) { return DummyMethod(3); });
            //methods.Add(delegate(object obj) { return DummyMethod(4); });

            foreach (var method in methods)
            {
                int c = method(null);
                Console.WriteLine("In main method c = " + c);
            }
        }

Also if the following code is used, I get the desired result.

        for (int i = 0; i < 5; ++i)
        {
            int j = i;
            methods.Add(delegate(object obj) { return DummyMethod(j); });
        }
recursive
  • 83,943
  • 34
  • 151
  • 241
malay
  • 1,434
  • 4
  • 17
  • 28
  • 2
    Favouriting this question. This sort of behaviour just feels "dangerous". I bet its going to come up more often as one of those hard-to-debug issues as C# moves more & more towards a delegate-oriented way of encapsulating behaviour. This is like the looping equivalent of switch clauses being allowed to cascade downward without mandatory "break;" clause – Neil Fenwick Nov 02 '09 at 10:57
  • @Neil: I agree it's a gotcha-hotspot. It's more confusing when it's seen with "foreach" - so much so that the C# team is considering changing the behaviour for that case. (It's hard to think of a scenario where the foreach behaviour is the desirable one; the for loop situation is more understandable as it's clear that the variable is only declared once.) – Jon Skeet Nov 02 '09 at 11:18
  • possible duplicate of [C# Captured Variable In Loop](http://stackoverflow.com/questions/271440/c-sharp-captured-variable-in-loop) – nawfal Nov 02 '13 at 06:09

4 Answers4

17

The problem is that you're capturing the same variable i in every delegate - which by the end of the loop just has the value 5.

Instead, you want each delegate to capture a different variable, which means declaring a new variable in the loop:

for (int i = 0; i < 5; ++i)
{
    int localCopy = i;
    methods.Add(delegate(object obj) { return DummyMethod(localCopy); });
}

This is a pretty common "gotcha" - you can read a bit more about captured variables and closures in my closures article.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • 1
    Jon, trying to get my head around why this happens. I looked at "i" as a value-type thinking "thats got to be passed as a copy", so I couldn't see how it seemed to have reference-like behaviour... You got a summary / one-liner to point me in right direction? – Neil Fenwick Nov 02 '09 at 10:48
  • 2
    Have a look at the final link. The thing to get your head round is that it's not the variable's *value* which is captured - it's the variable itself. – Jon Skeet Nov 02 '09 at 10:50
  • 1
    Think about this: It's like there's a little inline event that's called right in that spot in the method and thus has access to all of the variables and state, only you get the variables and state when the method's called not created. Thus you're simply accessing i when the method is called not when you make the anonymous method. – RCIX Nov 02 '09 at 10:57
  • @Neil I've put the "behind-the-scenes" code in my answer below. This is what helped me understand all this. – Matt Warren Nov 02 '09 at 11:01
4

This article will probably help you understand what is happening (i.e. what a closure is): http://blogs.msdn.com/oldnewthing/archive/2006/08/02/686456.aspx

Dan Byström
  • 9,067
  • 5
  • 38
  • 68
4

If you look at the code generated (using Reflector) you can see the difference:

private static void Method2()
{
    List<Methodx> list = new List<Methodx>();
    Methodx item = null;
    <>c__DisplayClassa classa = new <>c__DisplayClassa();
    classa.i = 0;
    while (classa.i < 5)
    {
        if (item == null)
        {
            item = new Methodx(classa.<Method2>b__8);
        }
        list.Add(item);
        classa.i++;
    }
    foreach (Methodx methodx2 in list)
    {
        Console.WriteLine("In main method c = " + methodx2(null));
    }
}

When you use the initial code it creates a temporary class in the background, this class holds a reference to the "i" variable, so as per Jon's answer, you only see the final value of this.

private sealed class <>c__DisplayClassa
{
    // Fields
    public int i;

    // Methods
    public <>c__DisplayClassa();
    public int <Method2>b__8(object obj);
}

I really recommend looking at the code in Reflector to see what's going on, its how I made sense of captured variables. Make sure you set the Optimization of the code to ".NET 1.0" in the Option menu, otherwise it'll hide all the behind scenes stuff.

Matt Warren
  • 10,279
  • 7
  • 48
  • 63
2

I think it is because the variable i is put to the heap (it's a captured variable)

Take a look at this answer.

Community
  • 1
  • 1
Stefan Steinegger
  • 63,782
  • 15
  • 129
  • 193