Reversing an ienumerable of lambda functions

Question

I have created an ienumerable of lambda functions using this function

    static IEnumerable<Func<int>> MakeEnumerator(int[] values)
    {
        for (int a = 0; a < values.Length; a++)
        {
            yield return () => Values[a];
        }
    }

I cannot then reverse this using LINQ or convert into an array without all the values becoming the last function. Example code (note this just demonstrates the problem it is not the code in the application):

        int[] start = {1,2,3};

        IEnumerable<Func<int>> end = MakeEnumerator(start).Reverse<Func<int>>();

        foreach (Func<int> i in end)
        {
            Console.WriteLine(i());
        }

I think the problem is in the MakeEnumerator function. How would I modify this to make it work or go about writing a working replacement reverse function.

Code inspection tools like Resharper will show a warning and often a proposed solution in this situation. — Gert Arnold, Sep 16 '11 at 15:04

score 5 · Accepted Answer · answered Sep 16 '11 at 14:23

5

The problem is that you're capturing the loop variable. All of your delegates are capturing the same variable, so they'll always see the latest value of a... which will be values.Length + 1 by the time you're executing the delegates, in your use cases. You can simply copy it instead:

for (int a = 0; a < values.Length; a++)
{
    int copy = a;
    yield return () => Values[copy];
}

Alternatively (and preferrably IMO) use a foreach loop, which currently requires the same workaround:

foreach (int value in values)
{
    int copy = value;
    yield return () => copy;
}

Or better yet:

return values.Select(x => (Func<int>)(() => x));

Or:

Func<int, Func<int>> projection = x => () => x;
return values.Select(projection);

See Eric Lippert's blog post "Closing over the loop variable considered harmful" for more information. Note that the behaviour of foreach may well be changing for C# 4.5.

answered Sep 16 '11 at 14:23

Jon Skeet

1,421,763
867
9,128
9,194

Is there anything solid (aka something in writing somewhere) yet in regards to the planned changes for `foreach`? – BrokenGlass Sep 16 '11 at 14:28
@BrokenGlass: Eric Lippert has made reasonably confident predictions that it will be changing, in comments here on Stack Overflow. I haven't seen anything definitive in a blog post though. – Jon Skeet Sep 16 '11 at 14:31
Why did you not simply write `yield return () => value;`? – Nawaz Sep 16 '11 at 14:37
Have you seen anything about `foreach` changing besides http://stackoverflow.com/questions/7350495/whats-going-on-behind-the-scene-of-the-foreach-loop/7350562#7350562? – Gabe Sep 16 '11 at 14:43
@Nawaz: That would suffer the same problem - it would be the `value` variable which was captured; the same variable for all iterations. – Jon Skeet Sep 16 '11 at 15:03
@Gabe: Just other comments, usually on my answers. – Jon Skeet Sep 16 '11 at 15:04
@Jon: Wouldn't it return a *copy* of value? Why? Have you covered this in your book? I want to read this in detail. – Nawaz Sep 16 '11 at 15:05
@Nawaz: Have you tried following the link at the bottom of the answer? I do cover it in the book, I believe, but Eric's post goes into more detail. – Jon Skeet Sep 16 '11 at 15:08
@Jon: Ohh.. you've edited the answer, making it more perfect. Thanks. I'll read that. +1. – Nawaz Sep 16 '11 at 15:10

score 2 · Answer 2 · answered Sep 16 '11 at 14:23

All of your lambda expressions are sharing the same a variable.
Since you're only calling them after the loop finishes, a is always 3.

You need to give each one its own variable:

for (int dontUse = 0; dontUse < values.Length; dontUse++)
{
    int a = dontUse;
    yield return () => Values[a];
}

In this code, each lambda expression gets its own a variable (since it's scoped inside the loop), and these separate variables never change.

Reversing an ienumerable of lambda functions

2 Answers2