Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Should IEquatable<T>, IComparable<T> be implemented on non-sealed classes?

Anyone have any opinions on whether or not IEquatable<T> or IComparable<T> should generally require that T is sealed (if it's a class)?

This question occurred to me since I'm writing a set of base classes intended to aid in the implementation of immutable classes. Part of the functionality which the base class is intended to provide is automatic implementation of equality comparisons (using the class's fields together with attributes which can be applied to fields to control equality comparisons). It should be pretty nice when I'm finished - I'm using expression trees to dynamically create a compiled comparison function for each T, so the comparison function should be very close to the performance of a regular equality comparison function. (I'm using an immutable dictionary keyed on System.Type and double check locking to store the generated comparison functions in a manner that's reasonably performant)

One thing that has cropped up though, is what functions to use to check equality of the member fields. My initial intention was to check if each member field's type (which I'll call X) implements IEquatable<X>. However, after some thought, I don't think this is safe to use unless X is sealed. The reason being that if X is not sealed, I can't know for sure if X is appropriately delegating equality checks to a virtual method on X, thereby allowing a subtype to override the equality comparison.

This then brings up a more general question - if a type is not sealed, should it really implement these interfaces AT ALL?? I would think not, since I would argue that the interfaces contract is to compare between two X types, not two types which may or may not be X (though they must of course be X or a subtype).

What do you guys think? Should IEquatable<T> and IComparable<T> be avoided for unsealed classes? (Also makes me wonder if there is an fxcop rule for this)

My current thought is to have my generated comparison function only use IEquatable<T> on member fields whose T is sealed, and instead to use the virtual Object.Equals(Object obj) if T is unsealed even if T implements IEquatable<T>, since the field could potentially store subtypes of T and I doubt most implementations of IEquatable<T> are designed appropriately for inheritance.

like image 439
Phil Avatar asked Dec 08 '09 16:12

Phil


3 Answers

I've been thinking about this question for a bit and after a bit of consideration I agree that implementing IEquatable<T> and IComparable<T> should only be done on sealed types.

I went back and forth for a bit but then I thought of the following test. Under what circumstances should the following ever return false? IMHO, 2 objects are either equal or they are not.

public void EqualitySanityCheck<T>(T left, T right) where T : IEquatable<T> {
  var equals1 = left.Equals(right);
  var equals2 = ((IEqutable<T>)left).Equals(right);
  Assert.AreEqual(equals1,equals2);
}

The result of IEquatable<T> on a given object should have the same behavior as Object.Equals assuming the comparer is of the equivalent type. Implementing IEquatable<T> twice in an object hierarchy allows for, and implies, there are 2 different ways of expressing equality in your system. It's easy to contrive any number of scenarios where IEquatable<T> and Object.Equals would differ since there are multiple IEquatable<T> implementations but only a single Object.Equals. Hence the above would fail and create a bit of confusion in your code.

Some people may argue that implementing IEquatable<T> at a higher point in the object hierarchy is valid because you want to compare a subset of the objects properties. In that case you should favor an IEqualityComparer<T> which is specifically designed to compare those properties.

like image 65
JaredPar Avatar answered Nov 20 '22 09:11

JaredPar


I would generally recommend against implementing IEquatable<T> on any non-sealed class, or implementing non-generic IComparable on most, but the same cannot be said for IComparable<T>. Two reasons:

  1. There already exists a means of comparing objects which may or may not be the same type: Object.Equals. Since IEquatable<T> does not include GetHashCode, its behavior essentially has to match that of Object.Equals. The only reason for implementing IEquatable<T> in addition to Object.Equals is performance. IEquatable<T> offers a small performance improvement versus Object.Equals when applied to sealed class types, and a big improvement when applied to structure types. The only way an unsealed type's implementation of IEquatable<T>.Equals can ensure that its behavior matches that of a possibly-overridden Object.Equals, however, is to call Object.Equals. If IEquatable<T>.Equals has to call Object.Equals, any possible performance advantage vanishes.
  2. It is sometimes possible, meaningful, and useful, for a base class to have a defined natural ordering involving only base-class properties, which will be consistent through all subclasses. When checking two objects for equality, the result shouldn't depend upon whether one regards the objects as being a base type or a derived type. When ranking objects, however, the result should often depend upon the type being used as the basis for comparison. Derived-class objects should implement IComparable<TheirOwnType> but should not override the base type's comparison method. It is entirely reasonable for two derived-class objects to compare as "unranked" when compared as the parent type, but for one to compare above the other when compared as the derived type.

Implementation of non-generic IComparable in inheritable classes is perhaps more questionable than implementation of IComparable<T>. Probably the best thing to do is allow a base-class to implement it if it's not expected that any child class will need some other ordering, but for child classes not to reimplement or override parent-class implementations.

like image 5
supercat Avatar answered Nov 20 '22 09:11

supercat


Most Equals implementations I've seen check the types of the objects being compared, if they aren't the same then the method returns false.

This neatly avoids the problem of a sub-type being compared against it's parent type, thereby negating the need for sealing a class.

An obvious example of this would be trying to compare a 2D point (A) with a 3D point (B): for a 2D the x and y values of a 3D point might be equal, but for a 3D point, the z value will most likely be different.

This means that A == B would be true, but B == A would be false. Most people like the Equals operators to be commutative, to checking types is clearly a good idea in this case.

But what if you subclass and you don't add any new properties? Well, that's a bit harder to answer, and possibly depends on your situation.

like image 1
ninj Avatar answered Nov 20 '22 07:11

ninj