Building a LINQ query programatically without local variables tricking me

Question

Fascinating (to me) situation..

Assume my objects are in perfect working order (i.e. TDD makes me think they work).

I have a list that I create like this (except indented properly):

var result = from v in vendors
             from p in v.Products
             orderby p.Name
             select p;

This works - I get all products from all vendors.

Now I have a list of conditions, built up at runtime by the user. Let's apply them:

foreach (Attribute a in requiredAttributes)
{
    result = result.Where(p => p.Attributes.Contains(a));
}

This may be primitive, but I thought it'd work. However, after this foreach loop is finished, when you enumerate "result" it will contain all products that has the LAST Attribute of the requiredAttributes collection in it's Attributes property (also a collection).

To me, this smells like "a" is overwritten somewhere with each trip trough the loop, and only the last one applies.

Short of somehow writing an extension method to IEnumerable called ContainsAll(IEnumerable) or something to that effect, how can I achieve what I want, which is basically a logical AND, giving me only those products that has ALL the required attributes?

Thanks!

Answer 1

var result = from v in vendors
             from p in v.Products
             where requiredAttributes.All(a => p.Attributes.Contains(a))
             orderby p.Name
             select p;

HTH.

Answer 2

(Edited for clarity.)

The problem is the foreach loop, and the fact that the "a" variable is being captured and then changed each time. Here's a modification which will work, by effectively introducing a "new" variable for each iteration of the loop, and capturing that new variable.

foreach (Attribute a in requiredAttributes)
{
    Attribute copy = a;
    result = result.Where(p => p.Attributes.Contains(copy));
}

Omer's solution is a cleaner one if you can use it, but this may help if your real code is actually more complicated :)

EDIT: There's more about the issue in this closures article - scroll down to "Comparing capture strategies: complexity vs power".

Answer 3

I haven't coded it up, but change

foreach (Attribute a in requiredAttributes){    
    result = result.Where(p => p.Attributes.Contains(a));
}

to

foreach (Attribute a in requiredAttributes){    
    Attribute b = a;
    result = result.Where(p => p.Attributes.Contains(b));
}

should fix it too, I think.