Can someone explain to me why this does not work?

Question

I've been looking at this piece of code. I realize that the reason why this will always throw the notsupportedexception is because temp is always equal to 20; however, I'd like someone to explain to me why temp is always equal to 20 and not the value that was set to temp in the loop.

{
    delegate int Del(int i);
    static event Del MyEvent;

    static void Main(string[] args)
    {
        for (int i = 0; i < 20; i++)
            MyEvent += a =>
            {
                int temp = i;
                if (a != temp) throw new NotSupportedException();
                return a * 2;

            };

        Console.WriteLine("C'est fini");
        Console.WriteLine(GetValue(5));

        Console.ReadLine();
    }

    static int GetValue(int arg)
    {
        foreach(Del myEvent in MyEvent.GetInvocationList())
        {
            try
            {
                return myEvent(arg);
            }
            catch(NotSupportedException)
            {
                continue;
            }
        }
        throw new NotSupportedException();
    }
}

It's because of closures and it's already answered very well here http://stackoverflow.com/questions/5438307/detailed-explanation-of-variable-capture-in-closures — BenCr, Mar 31 '14 at 21:30
is it because a is always not equal to i so it is throwing the exception and not making it to the return statement? — Ronnie, Mar 31 '14 at 21:31
@BenCr Thanks! I didn't know what it was called and I apologize for the duplicate. I'm reading the question you linked. — Aelphaeis, Mar 31 '14 at 21:33

Lee · Answer 1 · 2014-03-31T21:34:38.897

5

In your handler definition you are closing over the loop variable i, not the current value of i, so when you reach the end of the loop, i = 20 in all the registered handlers. You can fix it by copying i to another loop variable:

for (int i = 0; i < 20; i++)
{
    int temp = i;
    MyEvent += a =>
    {
        if (a != temp) throw new NotSupportedException();
        return a * 2;
    };
}

edited Mar 31 '14 at 21:34

answered Mar 31 '14 at 21:28

Lee

142,018
20
234
287

Is there a way I can change this behavior to use the value of i at the time the delegate is subscribed to the event? – Aelphaeis Mar 31 '14 at 21:29
@Aelphaeis - Yes, copy the current value inside the loop. See update. – Lee Mar 31 '14 at 21:31

score 3 · Accepted Answer · answered Mar 31 '14 at 21:30

i is being used in a closure. You have this loop:

    for (int i = 0; i < 20; i++)
        MyEvent += a =>
            {
                int temp = i;
                if (a != temp) throw new NotSupportedException();
                return a * 2;
            };

The lambda uses i, not the value of i. In other words, you're creating delegates here that actually refer to the variable i that is being changed by the loop. One simple way around this is to use a local variable to force the closure to capture a variable that isn't being modified:

    for (int i = 0; i < 20; i++)
    {
        var value = i;
        MyEvent += a =>
            {
                int temp = value;
                if (a != temp) throw new NotSupportedException();
                return a * 2;
            };
    }

At first glance, you might think this doesn't work around your issue. But the local variable value is actually a different variable for each iteration. When that variable is captured in the closure, it won't be updated by the loop.

score 1 · Answer 3 · answered Mar 31 '14 at 21:31

  for (int i = 0; i < 20; i++)
        MyEvent += a =>
        {
            int temp = i;
            if (a != temp) throw new NotSupportedException();
            return a * 2;

        };

I'd like someone to explain to me why temp is always equal to 20 and not the value that was set to temp in the loop.

The thing is, temp is not being set to anything in the loop. What you're doing is declaring a new anonymous method (using the args => {} syntax, or lambda expressions).

Your MyEvent delegate then aggregates pointers to all your 20 new anonymous methods.

These 20 methods won't be executed until you fire the event, which happens on this line:

return myEvent(arg);

By that time, i is 20. So temp will also be 20.

score 1 · Answer 4 · answered Mar 31 '14 at 21:31

1

Simply move temp outside of the closure.

for (int i = 0; i < 20; i++)
{
    int temp = i;
    MyEvent += a =>
    {
        if (a != temp) throw new NotSupportedException();
        return a * 2;
    };
}

answered Mar 31 '14 at 21:31

McAden

13,714
5
37
63

Can someone explain to me why this does not work?

4 Answers4