CODEWITHSUNDEEP
c#lambda
Steve
Steve

Reputation: 12044

Different behaviors with a for loop and a foreach loop with closures

I can't explain an issue I've run across. Basically I get a different answer if I use lambda syntax in a foreach loop than if I use it in a for loop. In the code below I register a delegate in a "dispatcher" class. I then later wrap the delegate on the way out in another delegate and return a list of these wrapped delegates. I then execute them. The expected output of executing the wrapped function list is 1,2. However I don't see that when I combine a lambda and a foreach loop.

This is not the code that is causing the problem, but the simplest case I could make to reproduce it. I would prefer not to discuss use cases of this, I'm more curious as to why I get behavior I'm not expecting. If I use the foreach loop below with the lambda syntax it fails. If I use the new Action() syntax and a foreach it works, if I use the lambda syntax in a for loop it works. Can anyone explain what is going on here. This has me really stumped.

    public class Holder
{
    public Holder(int ID, Dispatcher disp)
    {
        this.ID = ID;
        disp.Register(Something);
    }
    public int ID { get; set; }
    private void Something(int test) { Console.WriteLine(ID.ToString()); }
}

public class Dispatcher
{
    List<Action<int>> m_Holder = new List<Action<int>>();

    public void Register(Action<int> func)
    {
        m_Holder.Add(func);
    }

    public List<Action<int>> ReturnWrappedList()
    {
        List<Action<int>> temp = new List<Action<int>>();

        //for (int i = 0; i < m_Holder.Count; i++)      //Works - gives 1, 2
        //{
        //    var action = m_Holder[i];
        //    temp.Add(p => action(p));
        //}

        foreach (var action in m_Holder)
        {
            temp.Add(p => action(p)); //Fails - gives 2,2
            //temp.Add(new Action<int>(action)); Works - gives 1,2
        }

        return temp;
    }
}

class Program
{
    static void Main(string[] args)
    {
        var disp = new Dispatcher();
        var hold1 = new Holder(1, disp);
        var hold2 = new Holder(2, disp);
        disp.ReturnWrappedList().ForEach(p => p(1));
    }
}

Upvotes: 1

Views: 199

Answers (3)

LukeH
LukeH

Reputation: 269648

This is the infamous "closing over the loop variable" gotcha.

Upvotes: 4

Dan Bryant
Dan Bryant

Reputation: 27515

This is the classic issue of a captured closure with a scope that isn't what you expect. In the foreach, the action has outer scope, so the execution captures the last value of the loop. In the for case, you create the action in inner scope, so the closure is over the local value at each iteration.

Upvotes: 0

Rodney S. Foley
Rodney S. Foley

Reputation: 10340

Have you tried:

foreach (var action in m_Holder)
{
    var a = action;
    temp.Add(p => a(p));
}

Upvotes: 0

Related Questions