IEquatable Interface what to do when checking for

Question

I have implemented the IEquatable interface in a class with the following code.

        public bool Equals(ClauseBE other)
        {
            if (this._id == other._id)
            {
                return true;
            }
            return false;
        }

        public override bool Equals(Object obj)
        {
            if (obj == null)
            {
                return base.Equals(obj);
            }

            if (!(obj is ClauseBE))
            {
                throw new InvalidCastException("The 'obj' argument is not a ClauseBE object.");
            }

            return Equals(obj as ClauseBE);
        }

        public override int GetHashCode()
        {
            return this._id.GetHashCode();
        }

        public static bool operator ==(ClauseBE a, ClauseBE b)
        {
            // cast to object so we call the overloaded Equals function which appropriately checks when b is null.
            return a.Equals(b as object);
        }

        public static bool operator !=(ClauseBE a, ClauseBE b)
        {
            // cast to object so we call the overloaded Equals function which appropriately checks when b is null.
            return !a.Equals(b as object);
        }

This code work very well for most all cases. However, the following check throws an exception in the equality operator overload method because a is null and therefore does not have a Equals method.

if(this.Clause != null)
{

}

What is the standard way to solve this issue?

EDIT

I have gone to this, but it seems pretty cumbersome. I was hoping there was a more elegant way to accomplish this.

    public static bool operator ==(ClauseBE a, ClauseBE b)
    {
        if (a as object == null && b as object == null)
        {
            return true;
        }

        if ((a as object == null && b as object != null)
            || (b as object == null && a as object != null))
        {
            return false;
        }

        // cast to object so we call the overloaded Equals function which appropriately checks when b is null.
        return a.Equals(b as object);
    }

    public static bool operator !=(ClauseBE a, ClauseBE b)
    {
        if (a as object == null && b as object == null)
        {
            return false;
        }

        if((a as object == null && b as object != null)
            || (b as object == null && a as object != null))
        {
            return true;
        }

        // cast to object so we call the overloaded Equals function which appropriately checks when b is null.
        return !a.Equals(b as object);
    }

Solution

Thanks all. I got a lot of good tips from everyone, I really appreciate it. This is what I finally settled on, it's a lot more elegant than what I had started with. All code is the same except operator overloads.

public static bool operator ==(ClauseBE a, ClauseBE b)
{
    if (ReferenceEquals(a, null) && ReferenceEquals(b, null))
    {
        return true;
    }

    if (ReferenceEquals(a, null) || ReferenceEquals(b, null))
    {
        return false;
    }

    return a.Equals(b);
}

public static bool operator !=(ClauseBE a, ClauseBE b)
{
    return !(a == b);
}

Answer 1

I prefer to perform all the comparison logic in the Equals(T) method, and leave the "if this or that is null, else ..." in operator overloads to the framework.

The only tricky thing about overriding operator overloads is that you can no longer use those operators in your Equals implementation, for example to compare with null. Instead, object.ReferenceEquals can be used to achieve the same effect.

Following the TwoDPoint example in the MSDN Guidelines for Overriding Equals() and Operator == article, this is the pattern I generate when implementing value equality for types:

public override bool Equals( object obj ) {
  // Note: For value types, would use:
  // return obj is TwoDPoint && this.Equals( (TwoDPoint)obj );
  return this.Equals( obj as TwoDPoint );
}

public bool Equals( TwoDPoint other ) {
  // Note: null check not needed for value types.
  return !object.ReferenceEquals( other, null )
      && EqualityComparer<int>.Default.Equals( this.X, other.X )
      && EqualityComparer<int>.Default.Equals( this.Y, other.Y );
}

public static bool operator ==( TwoDPoint left, TwoDPoint right ) {
  // System.Collections.Generic.EqualityComparer<T> will perform the null checks 
  //  on the operands, and will call the Equals overload if necessary.
  return EqualityComparer<TwoDPoint>.Default.Equals( left, right );
}

public static bool operator !=( TwoDPoint left, TwoDPoint right ) {
  return !EqualityComparer<TwoDPoint>.Default.Equals( left, right );
}

The form above is the safest implementation, as it simply forwards the field equality checks to the framework and requires no knowledge of whether the fields overload the equality operators. It is perfectly fine to simplify this where you know the overload exists:

public bool Equals( TwoDPoint other ) {
  return !object.ReferenceEquals( other, null )
      && this.X == other.X
      && this.Y == other.Y;
}

You can also replace the EqualityComparer<T> calls in the operator overloads with calls to the static object.Equals method when comparing reference types, or when boxing value types does not matter:

public static bool operator ==( TwoDPoint left, TwoDPoint right ) {
  return object.Equals( left, right );
}

public static bool operator !=( TwoDPoint left, TwoDPoint right ) {
  return !object.Equals( left, right );
}

See also What is the best algorithm for an overridden GetHashCode? for implementing GetHashCode.

Answer 2

Other answers give good solutions to the general problem.

However, your own code can be simplified into a relatively simple solution ...

Firstly, at the start of your == operator you have this:

    // First test
    if (a as object == null && b as object == null)
    {
        return true;
    }

This qualifies as "working too hard".

If ClauseBE is a reference type, then you only need to compare with null - the "as object" is redundant; equally, if ClauseBE is a value type, then it can never be null.

Assuming that ClauseBE is a reference type (the most likely case), then you can simplify to this - note that we use Object.Equals() to avoid infinite recursion and a stack blowout.

    // First test
    if (Object.Equals(a, null) && Object.Equals(b, null))
    {
        return true;
    }

One useful shortcut is to use Object.ReferenceEquals() - which handles nulls for you.

So you could write this instead:

    // First test
    if (Object.ReferenceEquals(a, b))
    {
        return true;
    }

with the bonus that this also handles the case where a and b are the same exact object.

Once you get past the Object.ReferenceEquals() test, you know that a and b are different.

So your next test:

    // Second test
    if ((a as object == null && b as object != null)
        || (b as object == null && a as object != null))
    {
        return false;
    }

can be simplified - since you know that if a is null, b cannot be null, and so on.

    // Second test
    if (Object.Equals(a, null) || Object.Equals(b, null))
    {
        return false;
    }

If this test fails, then you know that a and b are different, and that neither is null. A good time to call your overridden Equals().

    // Use the implementation of Equals() for the rest
    return a.Equals(b as object);