3

I'm having problems with this simple code and I don't understand why c# behaves this way.

The problem seems to be that c# uses the Linq expression reference instead of the value when using Lists inside Lists.

On the loop for numbers I select the numbers based on the list, they all exist so all numbers should be added to the list {1,2,3}.

The behavior is ok when you see the output from the console it show {1,2,3} inside the numbers loop.

The problem is on the loop of the list, in here it seems that Linq only adds the last number to the list so it outputs {3,3,3}.

I know I don't need a list of ints inside the list but it's just to prove a point it's very weird, is this a known "bug"?

EDIT: It seems that this is how it is supposed to work in c# prior to 5.0. In C# 5.0 (VS2012+ compiler) this behavior has been modified to what I would expect

static void Main()
{
    var list = new List<IEnumerable<int>>();
    var numbers = new[] {1, 2, 3};
    var numbers2 = new[] {1, 2, 3};

    foreach (var number in numbers)
    {
        var result = from s in numbers2
                     where s == number
                     select s;

        Console.WriteLine(result.First()); // outputs {1,2,3}
        list.Add(result);
    }

    foreach (var num in list)
    {
        Console.WriteLine(num.First()); // outputs {3,3,3}
    }
}

Output

1 2 3 3 3 3

Xaruth
  • 4,034
  • 3
  • 19
  • 26
hjgraca
  • 1,695
  • 1
  • 14
  • 29
  • 2
    The output I get is 1 2 3 1 2 3 – Moho Oct 16 '13 at 12:50
  • @Teejay "By the way, in C# 5.0 (VS2012+ compiler) this behavior has been modified to what you expect." So it seems this is "fixed" in c# 5.0 – hjgraca Oct 16 '13 at 12:59
  • @weston well it seems that it's only "fixed" in c# 5, so before that you can see this happening. – hjgraca Oct 16 '13 at 13:01
  • @hjgraca I've tried 3.5, 4 and 4.5 works as you expect 1,2,3,1,2,3 – weston Oct 16 '13 at 13:02
  • 2
    VS2010 it does as you say and VS2012 it does 1,2,3,1,2,3. So is not related to C# version, but is related to compiler. You learn something new everyday. – weston Oct 16 '13 at 13:09
  • I was getting confused, c# version is independent of .net version. So VS2012 always compiles against C# 5 regardless of target .net plaftorm. – weston Oct 16 '13 at 13:25
  • Take a look at this [article](http://blogs.msdn.com/b/ericlippert/archive/2009/11/12/closing-over-the-loop-variable-considered-harmful.aspx). – Alessandro D'Andria Oct 16 '13 at 13:34

4 Answers4

8

This

list.Add(result);

does not do what you expect. It does not add a list of numbers. It adds a query. Queries are executed on demand. Since, in your final output loop number = 3, all three queries return 3.

If you want to add a list, force immediate execution of the query by appending ToList to it:

list.Add(result.ToList());
Heinzi
  • 167,459
  • 57
  • 363
  • 519
  • yes this is exactly what I don't understand, why at the end when we execute the query it only returns {3,3,3} and not {1,2,3}? – hjgraca Oct 16 '13 at 12:50
  • 1
    Because, in the end `number = 3`. Thus, the query `where s == number` only matches on `3`. When you declare the query, the current value of `number` does *not* get baked in. On the contrary: Every time you execute the query, it checks the *current* value of `number`. – Heinzi Oct 16 '13 at 12:52
  • You are absolutely right if I do list.Add(result.ToList()); it seems that the query is executed and the result is ok. But this is a definitely a weird behavior... – hjgraca Oct 16 '13 at 12:54
  • 1
    @hjgraca You think it is weird because (probably) you don't get used to the deferred-execution-ness nature of Linq. In other words, linq query is not get executed until it is necessary. – Alireza Oct 16 '13 at 12:59
2

Yes it's known. I believe it's fixed in c# 5.

You can declare a value inside foreach and then use that value in the Console.WriteLine to stop it just using the end value for num.

See this for a clearer explaination. The foreach identifier and closures

Community
  • 1
  • 1
stevepkr84
  • 1,637
  • 2
  • 13
  • 23
  • 1
    I would say **modified** rather then **fixed**. – Teejay Oct 16 '13 at 13:00
  • They thought this may mislead some coders, but IMHO I think the old behavior it's they way it's meant to be, and also can add some power tools to code. – Teejay Oct 16 '13 at 13:01
  • 1
    @Teejay: The old behavior exists because of a design decision of the C# 1.0 compiler, putting the declaration of the loop variable outside of the loop. Back then, there were no lambdas or LINQ and, thus, it made no difference. It is generally considered to be a mistake even by the C# developers themselves, see e.g. http://blogs.msdn.com/b/ericlippert/archive/2009/11/12/closing-over-the-loop-variable-considered-harmful.aspx. – Heinzi Oct 16 '13 at 13:11
  • @Heinzi Yes, I know well that blog post. And If you read it, you can read that *"[...] there are some very good reasons for not making this change [...]"* and 4 of those reasons, so my opinion is not that dumb maybe. – Teejay Oct 16 '13 at 13:43
  • @Heinzi This is my favourite: *"it would make the **foreach** semantics inconsistent with **for** semantics"* – Teejay Oct 16 '13 at 13:46
  • @Teejay: I just wanted to point out that this behavior was probably not "deliberately" designed this way. I apologize if I gave the impression that I considered your opinion dumb, this was never my intention! Of course, I agree that such a change must be weighted carefully. – Heinzi Oct 16 '13 at 13:49
  • @Heinzi No problem, I never thought you considered your opinion dumb :) I was just saying! – Teejay Oct 16 '13 at 14:11
2

When queries are executed, number == 3.

This is because the first foreach loop has already been executed and the last assignment to number is infact 3.

LINQ queries are executed deferred when necessary (for example when you call a ToList()).

This is not a weird result, that's the way it's meant to be.

By the way, in C# 5.0 (VS2012+ compiler) this behavior has been modified to what you expect.

Teejay
  • 7,210
  • 10
  • 45
  • 76
1

A linq expression is executed only when its results are needed.

So

var result = from s in numbers2
where s == number
select s;

will be executed when the value of result is needed. But if you want the expression to be evaluated immediately, you need to fetch results from it and create an object.

Like

var result = (from s in numbers2
where s == number
select s).ToList();

If you want to see a proper flow, then set a breakpoint on the line of "from s in numbers2...." and see when it gets hit...

hope it helps...

Virus
  • 2,445
  • 1
  • 14
  • 17