Why does Contains() return false but wrapping in a list and Intersect() returns true?

Problem:

When I use Contains() against an IEnumerable<T> of classes that correctly implement IEquatable and override GetHashCode it returns false. If I wrap the match target in a list and perform an Intersect() then the match works correctly. I would prefer to use Contains().

On IEnumerable.Contains() from MSDN:

Elements are compared to the specified value by using the default equality comparer

On EqualityComparer<T>.Default Property from MSDN:

The Default property checks whether type T implements the System.IEquatable generic interface and if so returns an EqualityComparer that uses that implementation. Otherwise it returns an EqualityComparer that uses the overrides of Object.Equals and Object.GetHashCode provided by T.

As far as I understand, implementing IEquatable<T> on my class should mean that the Equals method is used by the default comparer when trying to find a match. I want to use Equals because I want there to be only one way that two objects are the same, I do not want a strategy that the developers have to remember to put in.

What I find strange is that if I wrap the match target in a List and then perform an Intersect then the match is correctly found.

What am I missing? Do I have to create an equality comparer too, like the MSDN article? MSDN suggests that having IEquatable is enough and that it will wrap it for me.

Example Console App

NB: GetHashCode() from Jon Skeet here

using System;
using System.Collections.Generic;
using System.Linq;

namespace ContainsNotDoingWhatIThoughtItWould
{
    class Program
    {
        public class MyEquatable : IEquatable<MyEquatable>
        {
            string[] tags;

            public MyEquatable(params string[] tags)
            {
                this.tags = tags;
            }

            public bool Equals(MyEquatable other)
            {
                if (other == null)
                {
                    return false;
                }

                if (this.tags.Count() != other.tags.Count())
                {
                    return false;
                }
                var commonTags = this.tags.Intersect(other.tags);
                return commonTags.Count() == this.tags.Count();
            }

            public override int GetHashCode()
            {
                int hash = 17;
                foreach (string element in this.tags.OrderBy(x => x))
                {
                    hash = unchecked(hash * element.GetHashCode());
                }
                return hash;
            }
        }

        static void Main(string[] args)
        {
            // Two objects for the search list
            var a = new MyEquatable("A");
            var ab = new MyEquatable("A", "B");

            IEnumerable<MyEquatable> myList = new MyEquatable[] 
            { 
                a, 
                ab 
            };

            // This is the MyEquatable that we want to find
            var target = new MyEquatable("A", "B");

            // Check that the equality and hashing works
            var isTrue1 = target.GetHashCode() == ab.GetHashCode();
            var isTrue2 = target.Equals(ab);


            var isFalse1 = target.GetHashCode() == a.GetHashCode();
            var isFalse2 = target.Equals(a);

            // Why is this false?
            var whyIsThisFalse = myList.Contains(target);

            // If that is false, why is this true?
            var wrappedChildTarget = new List<MyEquatable> { target };

            var thisIsTrue = myList.Intersect(wrappedChildTarget).Any();
        }
    }
}

.NET 4.5 Fiddle Example

Jon Skeet
people
quotationmark

Okay - the problem is actually in the array implementation of ICollection<T>.Contains. You can see that simply like this:

static void Main(string[] args)
{
    var ab = new MyEquatable("A", "B");
    var target = new MyEquatable("A", "B");

    var array = new[] { ab };
    Console.WriteLine(array.Contains(target)); // False

    var list = new List<MyEquatable> { ab };
    Console.WriteLine(list.Contains(target));  // True

    var sequence = array.Select(x => x);
    Console.WriteLine(sequence.Contains(target)); // True
}

Enumerable.Contains delegates to ICollection<T>.Contains if the source implements ICollection<T>, which is why you were getting the array behaviour rather than the Enumerable.Contains "long-hand" implementation in your code.

Now ICollection<T>.Contains does say it's up the implementation to pick which comparer to use:

Implementations can vary in how they determine equality of objects; for example, List<T> uses Comparer<T>.Default, whereas Dictionary<TKey, TValue> allows the user to specify the IComparer<T> implementation to use for comparing keys.

But:

  • That documentation is broken already, as it should be talking about EqualityComparer<T> and IEqualityComparer<T>, not Comparer<T> and IEqualityComparer<T>
  • The decision of arrays to use a comparer which is neither explicitly specified nor the default EqualityComparer<T> seems very unnatural to me.

The solution is to override object.Equals(object):

public override bool Equals(object other)
{
    return Equals(other as MyEquatable);
}

It's generally pleasant to implement both IEquatable<T> and override object.Equals(object), for consistency. So while your code should work already in my view

people

See more on this question at Stackoverflow