C# Overriding Equals using "as" and specialized Method for Correctness, Flexibility and Performance

Question

I wondered about the best way to implement a correct, flexible and fast Equals in C#, that can be used for practically any class and situation. I figured that a specialized Equals (taking an object of the actual class as parameter) is needed for performance. To avoid code duplication, the general Equals ought to call the specialized Equals. Null checks should be performed only once, even in inherited classes.

I finally came up with this design:

class MyClass
{
    public Int32 SomeValue1 = 1;
    public Int32 SomeValue2 = 25;

    // Ignoring GetHashCode for simplicity.

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

    public bool Equals(MyClass obj)
    {
     if (obj == null) {
      return false;
     }

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

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

     return true;
    }
}

class MyDerivedClass : MyClass
{
    public Int32 YetAnotherValue = 2;

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

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

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

     return true;
    }
}

Important ideas:

• Usage of the "as" operator. This way we don't have to check for nulls in the general Equals. Wrong class types get reduced to null and will be sorted out in the specialized Equals.
• Null check at exactly one point, even for derived classes.
• Checking the attributes one after another provides a clear structure.

Are there flaws in this concepts, or did i miss any conditions?

Answer 1

Your Equals method isn't reflexive when different types are involved:

MyDerivedClass mdc = new MyDerivedClass();
MyClass mc = new MyClass();
Object omdc = mdc;
Object omc = mc;

// mc.Equals(mdc) - true
// mdc.Equals(mc) - true by calling the right overload
// omc.Equals(omdc) - true
// omdc.Equals(omc) - false, the "as" in MyDerivedClass will result in null

The normal way round this is to use:

if (GetType() != other.GetType())
{
    return false;
}

See the docs in Object.Equals: "x.Equals(y) returns the same value as y.Equals(x)." Relying on overloading to give different results could end up with horribly issues which would be very subtle to debug.

Answer 2

After implementing Jon's ideas, the code looks like this:

class MyClass
{
    public Int32 SomeValue1 = 1;
    public Int32 SomeValue2 = 25;

    // Ignoring GetHashCode for simplicity.

    public override bool Equals(object obj)
    {
     if (obj == null) {
      return false;
     }

     if (GetType () != obj.GetType ())
      return false;

     MyClass o = (MyClass) obj;

     if (!SomeValue1.Equals (o.SomeValue1)) {
      return false;
     }

     if (!SomeValue2.Equals (o.SomeValue2)) {
      return false;
     }

     return true;
    }
}

class MyDerivedClass : MyClass
{
    public Int32 YetAnotherValue = 2;

    public override bool Equals(object obj)
    {
     if (!base.Equals (obj)) {
      return false;
     }

     // won't fail since base checks for type equality
     MyDerivedClass o = (MyDerivedClass) obj;

     if (!YetAnotherValue.Equals (o.YetAnotherValue)) {
      return false;
     }

     return true;
    }
}

Still something wrong, or can i go ahead and replace all Equals in my code? :)