How can I improve this code: Inheritance and IEquatable<>

Question

This is an example about what I´m trying to do:

public class Foo : IEquatable<Foo>
{
    public bool Equals(Foo other)
    {
        Type type1 = this.GetType();
        Type type2 = other.GetType();

        if (type1 != type2)
            return false;

        if (type1 == typeof(A))
        {
            A a = (A)this;
            A b = (A)other;

            return a.Equals(b);
        }
        else if (type1 == typeof(B))
        {
            B c = (B)this;
            B d = (B)other;

            return c.Equals(d);
        }
        else
        {
            throw new Exception("Something is wrong");
        }
    }
}

public class A : Foo, IEquatable<A>
{
    public int Number1 { get; set; }
    public int Number2 { get; set; }

    public bool Equals(A other)
    {
        return this.Number1 == other.Number1 && this.Number2 == other.Number2;
    }
}

public class B : Foo, IEquatable<B>
{
    public int Number1 { get; set; }
    public int Number2 { get; set; }
    public int Number3 { get; set; }

    public bool Equals(B other)
    {
        return this.Number1 == other.Number1 && this.Number2 == other.Number2 && this.Number3 == other.Number3;
    }
}

But as you can see above, I'd have to use many conditionals 'if' to identify the real type. The problem is I have to use the base class. For example:

A a = new A();
Foo foo = a;

foo.Equals(another);

Why do you need `IEquatable` in the first place? It's not adding any value here. Just make sure the subtypes (sensibly) override `object.GetHashCode` and `object.Equals(object)` as well, in addition to the existing `IEquatable` implementation. Then you get virtual method dispatch for free, and it will work in many more situations. — Ani, Aug 13 '11 at 15:17
Hmm. These classes seems strange. What is the purpose of Foo since it has no properties? Also if B is just A with one more Number why doesn't B inherit from A and just add Number3 instead of from Foo? — alun, Aug 13 '11 at 15:18
@oscar.fimbres: Sure, but could you tell us why you need (or think you need) `IEquatable`? You appear to want everything to go 'through' the `Foo.Equals(Foo)` method.. — Ani, Aug 13 '11 at 15:19
@alun this is only an example.. I have many classes where its base class is Foo, and I've got a Foo class, so I need to check it out that there's no equal element. — oscar.fimbres, Aug 13 '11 at 15:38
@Ani Not really, I only want to eliminate the list of conditions to another simple way — oscar.fimbres, Aug 13 '11 at 15:38
This may be of interest: http://stackoverflow.com/questions/1868316/should-iequatablet-icomparablet-be-implemented-on-non-sealed-classes — alun, Aug 13 '11 at 15:51

Ani · Answer 1 · 2011-08-13T15:59:13.867

As a direct answer your question, you appear to implement IEquatable<Foo> by always deferring to the (concrete) sub-class's IEquatable<self> implementation. This would look something like:

(Bad code, for demonstration only)

// You need to specify what you want when this method is called on a 
// vanilla Foo object. I assume here that Foo is abstract. If not, please
// specify desired behaviour.
public bool Equals(Foo other)
{
    if (other == null || other.GetType() != GetType())
        return false;

    // You can cache this MethodInfo..
    var equalsMethod = typeof(IEquatable<>).MakeGenericType(GetType())
                                           .GetMethod("Equals");

    return (bool)equalsMethod.Invoke(this, new object[] { other });
}

But it really isn't clear why you need the equality comparisons to always go "through" the base-class's IEquatable<self> implementation.

The framework already has the virtual Equals method that will result in dispatching equality-calls to the appropriate method. In addition, EqualityComparar<T>.Default (which is used by most collection-types for making equality checks) already has the smarts to choose IEquatable<self>.Equals(self) or object.Equals(object)as appropriate.

Trying to create an implementation of equality in the base-class that just forwards the request is adding no value to anything, as far as I can see.

Without further explanation on why you need the base-class IEquatable<> implementation, I recommend just implementing equality properly on each type. For example:

public class A : Foo, IEquatable<A>
{
    public int Number1 { get; set; }
    public int Number2 { get; set; }

    public bool Equals(A other)
    {
        return other != null 
            && Number1 == other.Number1
            && Number2 == other.Number2;
    }

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

    public override int GetHashCode()
    {
        return Number1 ^ Number2;
    }
}

I think you missed a call to generic Equals in your non-generic Equals — alun, Aug 13 '11 at 15:56

RockWorld · Answer 2 · 2011-08-13T15:38:16.520

Try this piece of code:

public class Foo : IEquatable<Foo>
{
    public virtual bool Equals(Foo other)
    {
        return true;
    }
}

public class A : Foo,IEquatable<A>
{
    public int Number1 { get; set; }
    public int Number2 { get; set; }

    public override bool Equals(Foo other)
    {
        if (other.GetType() == typeof(A))
        {
            return Equals((A)other);                
        }
        throw new InvalidOperationException("Object is not of type A");
    }
    public bool Equals(A other)
    {
        return this.Number1 == other.Number1 && this.Number2 == other.Number2;
    }
}

public class B : Foo,IEquatable<B>
{
    public int Number1 { get; set; }
    public int Number2 { get; set; }
    public int Number3 { get; set; }

    public override bool Equals(Foo other)
    {
        if (other.GetType() == typeof(B))
        {
            return Equals((B)other);

        }
        throw new InvalidOperationException("Object is not of type B");
    }
    public bool Equals(B other)
    {
        return this.Number1 == other.Number1 && this.Number2 == other.Number2 && this.Number3 == other.Number3;
    }
}

Note : You can use Assert functionality to do typechecking.

It looks fine, at the beggining I was thinking in establish the Foo class like abstract. I like this way, it seems strange virtual in Foo class, is possible to replace Foo class to interface? — oscar.fimbres, Aug 23 '11 at 20:19

score 0 · Answer 3 · answered Aug 13 '11 at 15:48

One option is to move the Number1 and Number2 properties to the base class, and only compare the member added to the subclass in the subclasses' equality methods.

class Foo
{
    // move the common properties to the base class
    public int Number1 { get; set; }
    public int Number2 { get; set; }

    public override bool Equals(object obj)
    {
        Foo objfoo = obj as Foo;
        return 
            objfoo != null
            // require objects being compared to be of
            // the same derived type (optionally)
            && this.GetType() == obj.GetType()
            && objfoo.Number1 == this.Number1
            && objfoo.Number2 == this.Number2;
    }
    public override int GetHashCode()
    {
        // xor the hash codes of the elements used to evaluate
        // equality
        return Number1.GetHashCode() ^ Number2.GetHashCode();
    }
}

class A : Foo, IEquatable<A>
{
    // A has no properties Foo does not.  Simply implement
    // IEquatable<A>

    public bool Equals(A other)
    {
        return this.Equals(other);
    }

    // can optionally override Equals(object) and GetHashCode()
    // to call base methods here
}

class B : Foo, IEquatable<B>
{
    // Add property Number3 to B
    public int Number3 { get; set; }
    public bool Equals(B other)
    {
        // base.Equals(other) evaluates Number1 and Number2
        return base.Equals(other)
            && this.Number3 == other.Number3;
    }
    public override int GetHashCode()
    {
        // include Number3 in the hashcode, since it is used
        // to evaluate equality
        return base.GetHashCode() ^ Number3.GetHashCode();
    }
    public override bool Equals(object obj)
    {
        return this.Equals(obj as B);
    }
}

score 0 · Answer 4 · answered Sep 20 '11 at 11:47

I think that derived classes should not be handled in base classes. Usually, "Foo" will know nothing about A and B.

It's still possible to make the base IEquatable implementation virtual, allowing A and B to override it and perform their specific equality checks, even if both equality checking and checked instance are available only as "Foo" or "Object".

That would treat .Equals(Foo obj) like a more specific form of Object.Equals(Object obj).

How can I improve this code: Inheritance and IEquatable<>

4 Answers4