Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

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

Tags:

c#

wpf

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);
like image 439
oscar.fimbres Avatar asked Aug 13 '11 14:08

oscar.fimbres


1 Answers

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;
    }
}
like image 113
Ani Avatar answered Sep 29 '22 20:09

Ani