score:6

Accepted answer

i have only glanced at your question, but i am 90% sure that you should read the first section of on lambdas, capture, and mutability (which includes links to 5 similar so questions) and all will become clear.

the basic gist of it is that the variable ofilter in your example has been captured in the closure by reference and not by value. that means that once the loop finishes iterating, the variable's reference is to the last one, so the value as evaluated at lambda execution time is the final one as well.

the cure is to insert a new variable inside the foreach loop whose scope is only that iteration rather than the whole loop:

foreach(var ofilter in filterlist)
{
    var filter = ofilter; // add this
    switch (ofilter.column) // this doesn't have to change, but can for consistency
    {
        case filtertype.supplier:
            query = query.where(p => p.supplierid == filter.id); // use `filter` here
            break;

now each closure is over a different filter variable that is declared anew inside of each loop, and your code will run as expected.

score:-3

the problem is that you're not appending to the query, you're replacing it each time through the foreach statement.

you want something like the predicatebuilder - http://www.albahari.com/nutshell/predicatebuilder.aspx

score:0

i think this is the clearest explanation i've ever seen: http://blogs.msdn.com/ericlippert/archive/2009/11/12/closing-over-the-loop-variable-considered-harmful.aspx:

basically, the problem arises because we specify that the foreach loop is a syntactic sugar for

{
    ienumerator<int> e = ((ienumerable<int>)values).getenumerator();
    try
    {
        int m; // outside the actual loop
        while(e.movenext())
        {
            m = (int)(int)e.current;
            funcs.add(()=>m);
        }
    }
    finally
    {
        if (e != null) ((idisposable)e).dispose();
    }
}

if we specified that the expansion was

try
{
    while(e.movenext())
    {
        int m; // inside
        m = (int)(int)e.current;
        funcs.add(()=>m);
    }

then the code would behave as expected.

score:2

working as designed. the issue you are confronting is the clash between lexical closure and mutable variables.

what you probably want to do is

foreach(var ofilter in filterlist)
{
    var o = ofilter;
    switch (o.column)
    {
        case filtertype.supplier:
            query = query.where(p => p.supplierid == o.id);
            break;
        case filtertype.category:
            query = query.where(p => p.categoryid == o.id);
            break;
        default:
            break;
    }
}

when compiled to il, the variable ofilter is declared once and used multiply. what you need is a variable declared separately for each use of that variable within a closure, which is what o is now there for.

while you're at it, get rid of that bastardized hungarian notation :p.


Related Query

More Query from same tag