tags:

views:

355

answers:

5

Implementing Equals() for reference types is harder than it seems. My current canonical implementation goes like this:

public bool Equals( MyClass obj )
{
  // If both refer to the same reference they are equal.
  if( ReferenceEquals( obj, this ) )
    return true;

  // If the other object is null they are not equal because in C# this cannot be null.
  if( ReferenceEquals( obj, null ) )
   return false;

   // Compare data to evaluate equality    
   return _data.Equals( obj._data );
}

public override bool Equals( object obj )
{
  // If both refer to the same reference they are equal.
  if( ReferenceEquals( obj, this ) )
    return true;

  // If the other object is null or is of a different types the objects are not equal. 
  if( ReferenceEquals( obj, null ) || obj.GetType() != GetType() )
    return false;

  // Use type-safe equality comparison
  return Equals( (MyClass)obj );
}

public override int GetHashCode()
{
  // Use data's hash code as our hashcode  
  return _data.GetHashCode();
}

I think that this covers all corner (inheritance and such) cases but I may be wrong. What do you guys think?

+1  A: 

It depends on whether you're writing a value type or a reference type. For a sortable value type, I recommend this: http://www.peterritchie.com/Hamlet/Downloads/77.aspx

Peter Ritchie
I know that the implementation for value types is different. I did ask about reference types.
guardi
A: 

Concerning inheritance, I think you should just let the OO paradigm does its magic.

Specifically, the GetType() check should be removed, it might break polymorphism down the line.

chakrit
I disagree. Imagine that we have Apple and Orange classes which derive from the Fruit class. If we remove the GetType() check in the implementation of Equals in Fruit, we could compare Apples to Oranges unless every derived class properly overrides Equals() too. It could get messy very quickly.
guardi
A: 

I agree with chakrit, objects of different types should be allowed to be semantically equal if they have the same data or ID.

Personally, I use the following:

    public override bool Equals(object obj)
    {
        var other = obj as MyClass;
        if (other == null) return false;

        return this.data.Equals(other.data);
    }
Santiago Palladino
I disagree with you. This would allow us to compare Apples to Oranges.
guardi
The point is that maybe you do want them to be equal. The GetType check won't allow the use of proxies, for example; you'll have 2 identical objects, one wrapped inside a container of a different type, but you want to identify them as the same.
Santiago Palladino
A: 

Better hope that this._data is not null if it's also a reference type.

public bool Equals( MyClass obj )
{
    if (obj == null) {
     return false;
    }
    else {
     return (this._data != null && this._data.Equals( obj._data ))
                         || obj._data == null;
    }
}

public override bool Equals( object obj )
{
    if (obj == null || !(obj is MyClass)) {
     return false;
    }
    else {
     return this.Equals( (MyClass)obj );
    }
}

public override int GetHashCode() {
    return this._data == null ? 0 : this._data.GetHashCode();
}
tvanfosson
You are right. This is just a "canonical" implementation to prove the concepts behind "Equals". My "real" implementation usually is implemented using Equals(a,b) instead of a.Equals(b).
guardi
+3  A: 

I wrote a fairly comprehensive guide to this a while back. For a start your equals implementations should be shared (i.e. the overload taking an object should pass through to the one taking a strongly typed object). Additionally you need to consider things such as your object should be immutable because of the need to override GetHashCode. More info here:

http://gregbeech.com/blogs/tech/archive/2006/07/04/implementing-object-equality-in-net.aspx

My implementation is "shared". As you can see, at the end of Equals(Object) there is a call to Equals(MyClass).I am aware of the mutability and GetHashCode() issues; but it is a an important observation. It has bit me several times. Too bad that there's no "easy" way to declare read only "classes".
guardi