Most robust Equals implementation for custom classes for value equality in C#

Question

Say I have a Point2 class, and I want to implement the following Equals:

public override bool Equals ( object obj )

public bool Equals ( Point2 obj )

This is from the Effective C# 3 book:

public override bool Equals ( object obj )
{
    // STEP 1: Check for null
    if ( obj == null )
    {
        return false;
    }

    // STEP 3: equivalent data types
    if ( this.GetType ( ) != obj.GetType ( ) )
    {
        return false;
    }
    return Equals ( ( Point2 ) obj );
}

public bool Equals ( Point2 obj )
{
    // STEP 1: Check for null if nullable (e.g., a reference type)
    if ( obj == null )
    {
        return false;
    }
    // STEP 2: Check for ReferenceEquals if this is a reference type
    if ( ReferenceEquals ( this, obj ) )
    {
        return true;
    }
    // STEP 4: Possibly check for equivalent hash codes
    if ( this.GetHashCode ( ) != obj.GetHashCode ( ) )
    {
        return false;
    }
    // STEP 5: Check base.Equals if base overrides Equals()
    System.Diagnostics.Debug.Assert (
        base.GetType ( ) != typeof ( object ) );

    if ( !base.Equals ( obj ) )
    {
        return false;
    }

    // STEP 6: Compare identifying fields for equality.
    return ( ( this.X.Equals ( obj.X ) ) && ( this.Y.Equals ( obj.Y ) ) );
}

Is that an overkill?

Answer 1

Supporting equality with an inheritance hierarchy is tricky. You need to work out exactly what you mean. Do you really need inheritance here? If not - if Point2 derives directly from System.Object, and you can make it sealed, life becomes a bit easier. In that case I would use:

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

public bool Equals (Point2 obj)
{
    // STEP 1: Check for null if nullable (e.g., a reference type)
    // Note use of ReferenceEquals in case you overload ==.
    if (object.ReferenceEquals(obj, null))
    {
        return false;
    }

    // STEP 2: Check for ReferenceEquals if this is a reference type
    // Skip this or not? With only two fields to check, it's probably
    // not worth it. If the later checks are costly, it could be.
    if (object.ReferenceEquals( this, obj))
    {
        return true;
    }

    // STEP 4: Possibly check for equivalent hash codes
    // Skipped in this case: would be *less* efficient

    // STEP 5: Check base.Equals if base overrides Equals()
    // Skipped in this case

    // STEP 6: Compare identifying fields for equality.
    // In this case I'm using == instead of Equals for brevity
    // - assuming X and Y are of a type which overloads ==.
    return this.X == obj.X && this.Y == obj.Y;
}

Answer 2

Not really - you are accounting for pretty much every possibility. If this code is for anything other than a scratch application you should consider the benefits of this approach because logical errors due to weird object equality behavior are painful to debug.

Answer 3

Looks to me like exactly what you would want. That whole block boils down to:

"if it's the exact same instance, return true. If they're separate instances with the same X and Y values, return true. All other cases (null, different type, different x/y values) return false."

Answer 4

Its certainly more code than I'd want to write for an equals method. There are a lot of redundant checks such as checking for ReferenceEquals and HashCodes (I know these checks are redundant because the last lines in your function check for structural equality). Focus on code which is simple and readable.

public bool Equals(object o)
{
    Point2 p = o as Point2;
    if (o != null)
        return this.X == o.X && this.Y == o.Y;
    else
        return false;
}

Since your equals method uses structural equality, make sure you override GetHashCode with an implementation based on your fields as well.