Overriding equals in C# interface implementation

Question

I have a class that implements an interface, such as this:

interface IInterface
{
   string PropA { get; }
   string PropB { get; }
}

class AClass : IInterface
{
   string PropA { get; protected set; }
   string PropB { get; protected set; }
}

Equality is determined based on PropA and PropB. When overriding the Equals method for AClass, should I attempt to cast obj to AClass, like this:

public override bool Equals(object obj)
{
   AClass other = obj as AClass;
   return other != null 
       && AClass.PropA == other.PropA 
       && AClass.PropB == PropB;
}

Or should I attempt to cast obj to IInterface, like this:

public override bool Equals(object obj)
{
   IInterface other = obj as IInterface;
   return other != null 
       && AClass.PropA == other.PropA 
       && AClass.PropB == PropB;
}

Answer 1

You could do whichever you want. The two aren't the same, functionally, but which is "right" for you is something that we can't answer. If I have a BClass class that implements the same interface, and it has the same values for both properties, should it be equal to your AClass object? If yes, do the latter, if not, do the former.

Personally, I would find the latter concerning. Generally I find that if a class is going to implement its own personal definition of equality, other classes shouldn't be equal to it. One main reason is that it's preferable if equality is symetric. That is to say aclass.Equals(bclass) should return the same thing as bclass.Equals(aclass). Getting that behavior when you don't restrict equality to the same type is...hard. It requires cooperation of all related classes.

If you have some compelling reason to be comparing IInterface implementations in which they might be different underlying classes but still both be "equal", I'd personally prefer to create an IEqualityComparer<IInterface> that defines equality for that interface. This would the be separate from the definition of equality for either of the two implementing classes.

Answer 2

Decide how you need it to function.

Resharper implementation:

class AClass : IInterface, IEquatable<AClass>
{
    public bool Equals(AClass other)
    {
        if (ReferenceEquals(null, other)) return false;
        if (ReferenceEquals(this, other)) return true;
        return string.Equals(this.PropA, other.PropA) && string.Equals(this.PropB, other.PropB);
    }

    public override bool Equals(object obj)
    {
        if (ReferenceEquals(null, obj)) return false;
        if (ReferenceEquals(this, obj)) return true;
        if (obj.GetType() != typeof (AClass)) return false;
        return Equals((AClass)obj);
    }

    public override int GetHashCode()
    {
        unchecked
        {
            return ((this.PropA != null ? this.PropA.GetHashCode() : 0) * 397) ^ (this.PropB != null ? this.PropB.GetHashCode() : 0);
        }
    }

    public string PropA { get; protected set; }
    public string PropB { get; protected set; }
}

Answer 3

If the purpose of the interface is to hide from consumers the fact that two equivalent objects might be of different classes, it may be a good idea to define a struct which holds a single private field of that interface type and chains to the appropriate methods of the interfaces. Use of such a struct should generally be essentially as efficient as using a variable of the interface type (the main exception would be if the struct ends up being boxed), but would shield client code from seeing the actual type of thing implementing the interface.

For example, one might have a interfaces IReadableMatrix<T> and IImmutableMatrix<T>, and corresponding structures ReadableMatrix<T> and ImmutableMatrix<T> with read-only members int Height, int Width, and T this[int row, int column], and ImmutableMatrix<T> AsImmutable();. Code which uses an ImmutableMatrix<double> shouldn't care how it's stored; it would be entirely possible that two instances of ImmutableMatrix might hold references to different implementations of IImmutableMatrix<T> which report identical content in every cell, but stored things entirely differently. One might be an instance of ArrayBackedMatrix<double>, which holds a 12x12 array that happens to hold zeroes in every element other than those on the diagonal, while the other might be a DiagonalMatrix<double>, and use a 12-item array that only stores things on the diagonal (and returns zero in response to a request for any other element). The use of different types to store the array data should be an implementation detail and not exposed to the client.

One slight detail of using a struct to wrap the arrays is that reference-type fields of a default-value structure will be null, but the structure itself will not. It may thus be desirable to either have the struct implement an IsNull property which returns true if the backing field is null, or else have the other struct members check whether the backing field is null and, if so, behave as an empty 0x0 matrix.