IEquatable, how to implement this properly

Question

I am using .net 2.0 and c# and I have implemented the IEquatible interface in my class like this:-

public MyClass() :  IEquatable<MyClass>
{
    Guid m_id = Guid.NewGuid();

    public Guid Id
    {
        get
        {
            return m_id;
        }
    }

    #region IEquatable<MyClass> Members

    public bool Equals(MyClass other)
    {
        if (this.Id == other.Id)
        {
            return true;
        }
        else
        {
            return false;
        }
    }

    #endregion
}

Is this bad programming practice? I've read that I also need to implement Object.Equals and Object.GetHashCode as well, but I am not sure why.

I want to be able to check that an instance of MyClass is not already contained in a generic list of type MyClass. Why does the framework only suggests that you implement Equals only?

Any help would be greatly appreciated.

The implementation can be shortened to: `return this.Id == other.Id;`. — Oded, Jan 09 '12 at 11:06
Possible duplicate of [Is there a complete IEquatable implementation reference?](http://stackoverflow.com/questions/1307493/is-there-a-complete-iequatable-implementation-reference). Also, check [Understanding IEquatable](http://stackoverflow.com/questions/411500/understanding-iequatable). — vgru, Jan 09 '12 at 11:07
@AmarPalsapure - Same `NullReferenceException` that the code posted by the OP would produce. — Oded, Jan 09 '12 at 11:18
What are you trying to achieve? With the code posted above, each instance of MyClass will be initialised with a new Guid, which will mean that every instance is unique and not equatable to another instance, is this the desired behaviour? — DaveRead, Jan 09 '12 at 11:06
I agree. If each instance has a unique ID, then `Object.ReferenceEquals` will work the same way (unless there is a possibility to clone an instance of `MyClass`). — vgru, Jan 09 '12 at 11:11
Sorry perhaps my example is a bad one, say if I had a person class and I had 2 simple properties such as Name (string) and Age (int), using IEquatable, would I only have to implement Equals and test whether other's Name and Age are equal to this (the current instance), or is there more work to do? — bobbo, Jan 09 '12 at 11:39

score 2 · Answer 1 · edited May 23 '17 at 12:12

Is this bad programming practice?

Implementing IEquatable<T> is great, even more so for structs, but merely doing that much is not enough.

I've read that I also need to implement Object.Equals

Read it here why..

and Object.GetHashCode as well, but I am not sure why.

Read it here and here. Seriously, these have been discussed so many times, and it is pretty simple.. In short, you need it for collection types that deals with hashes like Dictionary<,> or HashSet<>

I want to be able to check that an instance of MyClass is not already contained in a generic list of type MyClass. Why does the framework only suggests that you implement Equals only?

Depends on the collection type. For a List<T>, it will check equality merely based on how you have defined Equals method, say for Contains method. For most scenario you will need Equals only. But if you have a HashSet<T> then absence and presence checks will utilize hash of your objects. Framework indeed asks us to implement good hashing approaches (without re-inventing the wheel) at appropriate places.

Any help would be greatly appreciated.

Do as below, but you have to overload operators == and != only if it make sense to you. Seeing your class I assumed its ok to have value semantics for your class. Otherwise just ignore that part (if == should mean reference equality)... Getting hashcode from your guid would suffice, provided that is all you need to test equality.

public sealed class MyClass : IEquatable<MyClass>
{
    Guid m_id = Guid.NewGuid();

    public Guid Id { get { return m_id; } }

    public bool Equals(MyClass other)
    {
        if (ReferenceEquals(this, other))
            return true;

        if (ReferenceEquals(null, other))
            return false;

        return Id == other.Id; 
    }

    public override bool Equals(object obj)
    {
        return Equals(obj as MyClass);
    }

    public static bool operator ==(MyClass lhs, MyClass rhs)
    {
        if (ReferenceEquals(lhs, null))
            return ReferenceEquals(rhs, null);

        return lhs.Equals(rhs);
    }

    public static bool operator !=(MyClass lhs, MyClass rhs)
    {
        return !(lhs == rhs);
    }

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

To not get it wrong, make use of the snippet available here: For a good overview see this SO thread.

score 2 · Answer 2 · answered Jan 09 '12 at 11:17

You can check if your list contains an item using a custom predicate for the criteria, using LINQ. In that case you don't need to override Equals nor implement IEquatable:

// check if the list contains an item with a specific ID
bool found = someList.Any(item => item.ID == someId);

Overriding Equals (with GetHashCode) and implementing IEquatable is useful if you need to store your item in a Dictionary or a Hashtable.

IEquatable, how to implement this properly

2 Answers2

Linked

Related