Why doesn't List.Contains work as I expect?

Question

Why does this program print "not added" while I think it should print "added"?

using System;
using System.Collections.Generic;

class Element
{
    public int id;

    public Element(int id)
    {
        this.id = id;
    }

    public static implicit operator Element(int d)  
    {
        Element ret = new Element(d);
        return ret;
    }

    public static bool operator ==(Element e1, Element e2)
    {
        return (e1.id == e2.id);
    }

    public static bool operator !=(Element e1, Element e2)
    {
        return !(e1.id == e2.id);
    }
}

class MainClass
{
    public static void Main(string[] args)
    {
        List<Element> element = new List<Element>();
        element.Add(2);
        if(element.Contains(2))
            Console.WriteLine("added");
        else
            Console.WriteLine("not added");
    }
}

The Contains method does not use the == operator‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌. What is the problem?

Answer 1

The Contains method does not use the == operator‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌

No - it uses Equals, which you haven't overridden... so you're getting the default behaviour of Equals, which is to check for reference identity instead. You should override Equals(object) and GetHashCode to be consistent with each other - and for sanity's sake, consistent with your == overload too.

I'd also recommend implementing IEquatable<Element>, which List<Element> will use in preference to Equals(object), as EqualityComparer<T>.Default picks it up appropriately.

Oh, and your operator overloads should handle null references, too.

I'd also strongly recommend using private fields instead of public ones, and making your type immutable - seal it and make id readonly. Implementing equality for mutable types can lead to odd situations. For example:

Dictionary<Element, string> dictionary = new Dictionary<Element, string>();
Element x = new Element(10);
dictionary[x] = "foo";
x.id = 100;
Console.WriteLine(dictionary[x]); // No such element!

This would happen because the hash code would change (at least under most implementations), so the hash table underlying the dictionary wouldn't be able to find even a reference to the same object that's already in there.

So your class would look something like this:

internal sealed class Element : IEquatable<Element>
{
    private readonly int id;

    public int Id { get { return id; } }

    public Element(int id)
    {
        this.id = id;
    }

    public static implicit operator Element(int d)  
    {
        return new Element(d);
    }

    public static bool operator ==(Element e1, Element e2)
    {
        if (object.ReferenceEquals(e1, e2))
        {
            return true; 
        }
        if (object.ReferenceEquals(e1, null) ||
            object.ReferenceEquals(e2, null))
        {
            return false; 
        }
        return e1.id == e2.id;
    }

    public static bool operator !=(Element e1, Element e2)
    {
        // Delegate...
        return !(e1 == e2);
    }

    public bool Equals(Element other)
    {
        return this == other;
    }

    public override int GetHashCode()
    {
        return id;
    }

    public override bool Equals(object obj)
    {
        // Delegate...
        return Equals(obj as Element);
    }
}

(I'm not sure about the merit of the implicit conversion, by the way - I typically stay away from those, myself.)

Answer 2

The Contains method does not use the == operator‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌‌. What is the problem?

That is correct.

This method [Contains] determines equality by using the default equality comparer, as defined by the object's implementation of the IEquatable.Equals method for T (the type of values in the list).

http://msdn.microsoft.com/en-us/library/bhkz42b3(v=vs.110).aspx

You need to override Equals() as well. Note when you overload Equals(), it is almost always correct to also override GetHashCode().