IEnumerable.Union(IEnumerable) overwrites contents instead of unioning

Question

I've got a collection of items (ADO.NET Entity Framework), and need to return a subset as search results based on a couple different criteria. Unfortunately, the criteria overlap in such a way that I can't just take the collection Where the criteria are met (or drop Where the criteria are not met), since this would leave out or duplicate valid items that should be returned.

I decided I would do each check individually, and combine the results. I considered using AddRange, but that would result in duplicates in the results list (and my understanding is it would enumerate the collection every time - am I correct/mistaken here?). I realized Union does not insert duplicates, and defers enumeration until necessary (again, is this understanding correct?).

The search is written as follows:

IEnumerable<MyClass> Results = Enumerable.Empty<MyClass>();
IEnumerable<MyClass> Potential = db.MyClasses.Where(x => x.Y); //Precondition

int parsed_key;

//For each searchable value
foreach(var selected in SelectedValues1)
{
    IEnumerable<MyClass> matched = Potential.Where(x => x.Value1 == selected);
    Results = Results.Union(matched); //This is where the problem is
}

//Ellipsed....

foreach(var selected in SelectedValuesN) //Happens to be integer
{
    if(!int.TryParse(selected, out parsed_id))
        continue;
    IEnumerable<MyClass> matched = Potential.Where(x => x.ValueN == parsed_id);
    Results = Results.Union(matched); //This is where the problem is
}

It seems, however, that Results = Results.Union(matched) is working more like Results = matched. I've stepped through with some test data and a test search. The search asks for results where the first field is -1, 0, 1, or 3. This should return 4 results (two 0s, a 1 and a 3). The first iteration of the loops works as expected, with Results still being empty. The second iteration also works as expected, with Results containing two items. After the third iteration, however, Results contains only one item.

Have I just misunderstood how .Union works, or is there something else going on here?

Answer 1

Because of deferred execution, by the time you eventually consume Results, it is the union of many Where queries all of which are based on the last value of selected.

So you have

Results = Potential.Where(selected)
    .Union(Potential.Where(selected))
    .Union(potential.Where(selected))...

and all the selected values are the same.

You need to create a var currentSelected = selected inside your loop and pass that to the query. That way each value of selected will be captured individually and you won't have this problem.

Answer 2

You can do this much more simply:

Reuslts = SelectedValues.SelectMany(s => Potential.Where(x => x.Value == s));

(this may return duplicates)

Or

Results = Potential.Where(x => SelectedValues.Contains(x.Value));

Answer 3

As pointed out by others, your LINQ expression is a closure. This means your variable selected is captured by the LINQ expression in each iteration of your foreach-loop. The same variable is used in each iteration of the foreach, so it will end up having whatever the last value was. To get around this, you will need to declare a local variable within the foreach-loop, like so:

//For each searchable value 
foreach(var selected in SelectedValues1) 
{
    var localSelected = selected;
    Results = Results.Union(Potential.Where(x => x.Value1 == localSelected));
}

It is much shorter to just use .Contains():

Results = Results.Union(Potential.Where(x => SelectedValues1.Contains(x.Value1)));

Since you need to query multiple SelectedValues collections, you could put them all inside their own collection and iterate over that as well, although you'd need some way of matching the correct field/property on your objects.

You could possibly do this by storing your lists of selected values in a Dictionary with the name of the field/property as the key. You would use Reflection to look up the correct field and perform your check. You could then shorten the code to the following:

// Store each of your searchable lists here
Dictionary<string, IEnumerable<MyClass>> DictionaryOfSelectedValues = ...;

Type t = typeof(MyType);
// For each list of searchable values
foreach(var selectedValues in DictionaryOfSelectedValues) // Returns KeyValuePair<TKey, TValue>
{
    // Try to get a property for this key
    PropertyInfo prop = t.GetProperty(selectedValues.Key);
    IEnumerable<MyClass> localSelected = selectedValues.Value;

    if( prop != null )
    {
        Results = Results.Union(Potential.Where(x =>
                localSelected.Contains(prop.GetValue(x, null))));
    }
    else // If it's not a property, check if the entry is for a field
    {
        FieldInfo field = t.GetField(selectedValues.Key);
        if( field != null )
        {
            Results = Results.Union(Potential.Where(x =>
                    localSelected.Contains(field.GetValue(x, null))));
        }
    }
}

Answer 4

No, your use of union is absoloutely correct. The only thing to keep in mind is it excludes duplicates as based on the equality operator. Do you have sample data?

Answer 5

Okay, I think you are are haveing a problem because Union uses deferred execution.

What happens if you do,

var unionResults = Results.Union(matched).ToList();
Results = unionResults;