Can't figure out why collection returns false for bool property?

Question

I'm running the following expression to get the delta between 2 collections based on the Id and also make sure the overall result includes only 'Active/Enabled' records:

//Returns the right records but all IsEnabled = false
var newRecords = allRecordsFromA.Where(a => !(allRecordsFromB.Any(b => ((b.Id == a.Id) && a.IsEnabled)))).ToList();

The above result returns all the right records in A that are not in B, but all of the records are marked IsEnabled = false. The really confusing part is I use the exact same expression on 2 other collections in another part of the application and it works. I get back all the proper records and they are all marked as .IsEnabled == true That makes no sense to me. I even copied over and it doesn't work.

However, if I change to a 2 step process I get the results I desire:

//Works
var newRecords = allRecordsFromA.Where(a => !(allRecordsFromB.Any(b => (b.Id == a.Id)))).ToList();
var results = newRecords.Where(x => x.IsEnabled).ToList();

I was thinking the order of operations in the 1st expression is wrong returning the records with the ! being applied to IsEnabled but I don't see it. What is incorrect in my 1st expression that is returning records with IsEnabled == false?

Answer 1

I think that your parentheses are not where you think they are. Specifically, a.IsEnabled is inside the Any statement, which is then negated.

allRecordsFromB.Any(b => ((b.Id == a.Id) && a.IsEnabled))

This gives you very different logic from your second example. I'd move a.IsEnabled to the start of your Where and remove some unnecessary parentheses to clarify things.

var newRecords = allRecordsFromA.Where(a => a.IsEnabled &&
                       !allRecordsFromB.Any(b => b.Id == a.Id)).ToList();

Note that if you have large data sets, or think the following approach would be clearer, you can speed things up by making a hash set of the b.Ids. (or via some sort of join, which internally would do the same sort of thing)

var bIds = new HashSet<int>(allRecordsFromB.Select(b => b.Id));
var newRecords = allRecordsFromA.Where(a => a.IsEnabled &&
                                            !bIds.Contains(a.Id)).ToList();

Answer 2

The check whether a is enabled is inside the Any, which is Negated. So if both the conditions b.Id == a.Id (for some b in allRecordsFromB) and a.IsEnabled are true, the element will not be part of the result. If one of the two is false, it will be part of the result.

So since that is the opposite of what you want, you should move the IsEnabled check outside of the Any.

Answer 3

I think part of the problem is that your Linq statement is hard to read and that kind of defeats the purpose. You may want to consider something like this:

var collectionA = new List<Record>();
var collectionB = new List<Record>();
var results = collectionA.Intersect(collectionB).Where(x => x.IsEnabled);

You have to implement the Equals method:

public class Record
{
    public int Id { get; set; }
    public bool IsEnabled { get; set; }

    protected bool Equals(Record other)
    {
        return Id == other.Id;
    }

    public override bool Equals(object obj)
    {
        if (ReferenceEquals(null, obj)) return false;
        if (ReferenceEquals(this, obj)) return true;
        if (obj.GetType() != this.GetType()) return false;
        return Equals((Record) obj);
    }

    public override int GetHashCode()
    {
        return Id;
    }
}

Or create a Comparer and pass that to the Intersect method:

public class RecordComparer : IEqualityComparer<Record>
{
    public bool Equals(Record x, Record y)
    {
        return x.Id == y.Id;
    }

    public int GetHashCode(Record obj)
    {
        return obj.GetHashCode();
    }
}