3

Once again discussing equality I stumbled on EqualityComparer<T>.Default.Equals(). I prefer to call this method for reference types rather than object.Equals().
Now I think I was dreadfully wrong.

object.Equals() uses overridable instance Equals() method providing correct polymorphic behavior whereas EqualityComparer<T>.Default.Equals() calls IEquatable<T>.Equals() if it's implemetned.

Now consider this small program:

public class Class1 : IEquatable<Class1>
{
    public int Prop1 { get; set; }

    public bool Equals(Class1 other)
    {
        if (other == null)
            return false;

        return Prop1 == other.Prop1;
    }

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

        return Equals(obj as Class1);
    }
}

public class Class2 : Class1, IEquatable<Class2>
{
    public int Prop1 { get; set; }
    public int Prop2 { get; set; }

    public bool Equals(Class2 other)
    {
        if (other == null)
            return false;

        return Prop1 == other.Prop1 && Prop2 == other.Prop2;
    }

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

        return Equals(obj as Class2);
    }
}


class Program
{
    static void Main(string[] args)
    {
        var c1 = new Class1 {Prop1 = 10};
        var c2 = new Class2 {Prop1 = 10, Prop2 = 5};
        var c3 = new Class2 {Prop1 = 10, Prop2 = 15};

        Console.WriteLine("Object.Equals()");
        Console.WriteLine("C1=C2 {0}",Equals(c1,c2));
        Console.WriteLine("C2=C1 {0}",Equals(c2, c1));
        Console.WriteLine("C2=C3 {0}",Equals(c2, c3));
        Console.WriteLine("C3=C2 {0}", Equals(c3, c2));

        var dec1 = EqualityComparer<Class1>.Default;

        Console.WriteLine();
        Console.WriteLine("EqualityComparer<Class1>.Default.Equals");
        Console.WriteLine("C1=C2 {0}", dec1.Equals(c1, c2));
        Console.WriteLine("C2=C1 {0}", dec1.Equals(c2, c1));
        Console.WriteLine("C2=C3 {0} BUG?", dec1.Equals(c2, c3));
        Console.WriteLine("C3=C2 {0} BUG?", dec1.Equals(c3, c2));

        Console.ReadKey();
    }
}

It shows how easy it is to bring inconsistency in equality semantics:

Object.Equals()
C1=C2 False
C2=C1 False
C2=C3 False
C3=C2 False

EqualityComparer<Class1>.Default.Equals
C1=C2 False
C2=C1 False
C2=C3 True BUG?
C3=C2 True BUG?

However MSDN Documentation recommdends:

Notes to Implementers If you implement Equals, you should also override the base class implementations of Object.Equals(Object) and GetHashCode so that their behavior is consistent with that of the IEquatable<T>.Equals method. If you do override Object.Equals(Object), your overridden implementation is also called in calls to the static Equals(System.Object, System.Object) method on your class. In addition, you should overload the op_Equality and op_Inequality operators. This ensures that all tests for equality return consistent results, which the example illustrates.

Starting from this moment I see no reason to implement IEquatable<T> for reference types. Can anyone tell me when it has any sense? Should I really treat different equality behavior as inconsistent when we look at the type differently (as base type)?

Dethariel
  • 3,394
  • 4
  • 33
  • 47
Pavel Voronin
  • 13,503
  • 7
  • 71
  • 137
  • `object.Equals()` checks if two variables point to the same object (reference equals), not whether two instances are equatable (e.g., have the same state). Children can override `Equals()`, yes, but calling `object.Equals(x, y)` is a different beast. –  Nov 26 '13 at 14:34
  • 1
    Personally what I do is have the `object.Equals()` override call `IEquatable.Equals()`. No technical reason beyond a fluffy "design cleanness" – objects simple are not "equatable" to any other object, which is why implementing an interface that specifies who you can be compared to makes some sense to me. – millimoose Nov 26 '13 at 14:34
  • 1
    @Will I call BS on the last claim. Did you look at the implementation of that method? – millimoose Nov 26 '13 at 14:35
  • 2
    You didn't make `object.Equals(object)` equivalent to `IEquatable.Equals(Class1)` in `Class1` like you should have (if you compare it to `Class2`, they don't always have the same result), so you see confusing results. The more common way of equating is to say that objects must have the exact same type to be considered equal (like your `Equals(object)` methods) – Tim S. Nov 26 '13 at 14:35
  • 1
    This is just a poor implementation of `IEquatable.Equals` in `Class1`, make the implementation virtual and override it in `Class2` – Lukazoid Nov 26 '13 at 14:36
  • @Will The docs for `object.Equals(x, y)` say that it **first** checks for reference equality, **then** checks for a null, **then** actually does call `x.Equals(y)`. For all intents and purposes it's just a convenient way to do the reference equality shortcut, and the `null` check. Unless, for some reason, you ever want to a) have an object inequal to itself, or b) equal to `null`, but I'm struggling to imagine when that would be useful. – millimoose Nov 26 '13 at 14:37
  • @Lukazoid To be fair it's a pretty flaky design if you require all your subclasses to override a non-abstract method. – millimoose Nov 26 '13 at 14:39
  • @millimoose Only as flaky as the design of `object.Equals(object)` which forces you to do the exact same thing – Lukazoid Nov 26 '13 at 14:41
  • @millimoose: The static implementation of Equals compares references. Protip: read the docs. Was trying to clarify the instance implementation is different or *can be* different via overriding than the static implementation. May not have been clear on that... –  Nov 26 '13 at 14:42
  • 1
    @Will ["If the two objects do not represent the same object reference and neither is null, it calls objA.Equals(objB) and returns the result."](http://msdn.microsoft.com/en-us/library/w4hkze5k(v=vs.110).aspx) Unless it is somehow possible for an object to be reference-equal to itself and yet not value-equal to itself, this will ultimately return the same results as just calling `x.Equals(y)` would've. – millimoose Nov 26 '13 at 14:43
  • @millimoose: Okay... yes, I see what you're saying. So, if the type overrides equals, then ... I herped. –  Nov 26 '13 at 14:44
  • @Will Yup. Basically, my point was that the instance implementation would have to be pretty exotic to yield different results in the end – millimoose Nov 26 '13 at 14:45
  • One weirdness that remains is that, by summarily calling `x.Equals(y)`, the code for `GenericEqualityComparer.Equals(T x, T y)` (the instance that is supplied by `EqualityComparer.Default` for generic `T`) is expressing an unmotivated ***arbitrary preference*** for the `x`, as opposed to `y`, override of the instance `Equals` method (still true in .NET 4.7). This seems like a hazard if the `x` instance at runtime happens to be less-derived than `y`, since the latter would be within its rights to ignore or alter the result(s) of any various base implementation override(s). Right? – Glenn Slayden Jun 04 '19 at 22:26
  • ...to be fair, it's not *necessarily* clear in principle what a *so-called* `EqualityComparer.Default`--*where the very name implies base behavior*--***should*** do in the face of a polymorphic hierarchy with competing derived-`Equals(TSelf x)` claims, but it seems like hard-coding the opaque and arbitrary (albeit predicable, if known) choice of `x` is poor design, as opposed to (e.g.,) instead detecting the lesser- (or more-) derived *instance* at runtime and accordingly calling ***its*** instance-`Equals` method. Doing so avoids introducing an `x` /`y` asymmetry, which seems more elegant. – Glenn Slayden Jun 04 '19 at 22:54
  • @GlennSlayden I remove polymorphism from this equation. I check types of `x` and `y`, if they are not same, then equality returns `false`. – Pavel Voronin Jun 05 '19 at 07:06

2 Answers2

3

Today I asked myself which consequences arise when adding IEquatable<T> to a class, and I found your question.
Then I tested your code. For everyone else reading this, here's an answer, instead of only "just do it like that to make it work".

First of all, it's not a bug.
Your problem is, that you specify an EqualityComparer<Class1>, which is only implemented in class1 by public bool Equals(Class1 other).
Therefore, dec1.Equals(c2, c3) will call this function where only the content of class1 is compared.

From your comment BUG? I can see that you expect the content of class2 to be compared as well, just like everybody else would expect, too. To achieve this, you need to change
public bool Equals(Class1 other)
into
public virtual bool Equals(Class1 other)
and override this function in class2, where you then also compare the content of class2.
But that may lead to a quite weird construct. Therefore, for completeness, here's my way of implementation:

In the base class, only type checks:

//--------------------------------------------------------------------------
public static bool operator == (CClass1 i_value1, CClass1 i_value2)
{
  if (ReferenceEquals (i_value1, i_value2))
    return true;
  if (ReferenceEquals (null, i_value1))
    return false;

  return (i_value1.Equals (i_value2));
}

//--------------------------------------------------------------------------
public static bool operator != (CClass1 i_value1, CClass1 i_value2)
{
  return !(i_value1 == i_value2);
}

///-------------------------------------------------------------------------
public sealed override bool Equals (object i_value)
{
  if (ReferenceEquals (null, i_value))
    return false;
  if (ReferenceEquals (this, i_value))
    return true;

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

  return Equals_EXEC ((CClass1)i_value);
}

///-------------------------------------------------------------------------
public bool Equals (CClass1 i_value)  // not virtual, don't allow overriding!
{
  if (ReferenceEquals (null, i_value))
    return false;
  if (ReferenceEquals (this, i_value))
    return true;

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

  return Equals_EXEC (i_value);
}

Still in the base class, content checks:

///-------------------------------------------------------------------------
protected override bool Equals_EXEC (CClass1 i_value)
{
  return Equals_exec (i_value);
}

//--------------------------------------------------------------------------
private bool Equals_exec (CClass1 i_value)
{
  return variable1 == i_value.variable1
      && variable2 == i_value.variable2
      && ... ;
}

In the derived classes, content checks:

///-------------------------------------------------------------------------
protected override bool Equals_EXEC (CClassN i_value)
{
  return base.Equals_EXEC (i_value)
      && Equals_exec (i_value as CClassN);
}

//--------------------------------------------------------------------------
private bool Equals_exec (CClassN i_value)
{
  return variable5 == i_value.variable5
      && variable6 == i_value.variable6
      && ... ;
}
Tobias Knauss
  • 3,361
  • 1
  • 21
  • 45
2

Rightly or wrongly, here is how I have tended to implement Equals(Object) and IEquatable<T>.Equals(T) on base and derived classes.

public class Class1 : IEquatable<Class1>
{    
    public sealed override bool Equals(object obj)
    {
        return Equals(obj as Class1);
    }

    public virtual bool Equals(Class1 obj)
    {
        if(ReferenceEquals(obj, null))
            return false;

        // Some property checking
    }
}

public class Class2 : Class1, IEquatable<Class2>
{
    public sealed override bool Equals(Class1 obj)
    {
        return Equals(obj as Class2);
    }

    public virtual bool Equals(Class2 obj)
    {
        if(!base.Equals(obj))
            return false;

        // Some more property checking
    }
}

public class Class3 : Class2, IEquatable<Class3>
{
    public sealed override bool Equals(Class2 obj)
    {
        return Equals(obj as Class3);
    }

    public virtual bool Equals(Class3 obj)
    {
        if(!base.Equals(obj))
            return false;

        // Some more property checking
    }
}

For reference types, the benefits of implementating IEquatable<T> are marginal, if you have two instances of type T, you are able to directly invoke T.Equals(T). instead of T.Equals(Object) which subsequently requires type checking to be performed on the parameter.

The primary purpose of IEquatable<T> is for value types, where there is overhead in boxing the instance.

Lukazoid
  • 19,016
  • 3
  • 62
  • 85
  • Well, at least your implimentation is more correct. But you need to override two methods instead of just one object.Equals(). So what is the profit of IEquatable here? – Pavel Voronin Nov 26 '13 at 14:53