Base class for implementing IComparable<T>

Question

If I create a class that implements IComparable<T>, I must implement CompareTo<T>. It is also recommended that I implement IEquatable<T> and the non-generic IComparable. If I do all that, I am required or encouraged to:

• Override GetHashCode()
• Implement CompareTo(Object)
• Override Equals(Object)
• Implement Operator ==(T, T)
• Implement Operator !=(T, T)
• Implement Operator >(T, T)
• Implement Operator <(T, T)
• Implement Operator >=(T, T)
• Implement Operator <=(T, T)

That's 9 additional methods, most of which depend on the logic that compares two instances of the class. Rather than having to implement all those methods in any class that implements IComparable<T>, I decided to create a base class that implements IComparable<T> and the other recommended interfaces (similar to the way Microsoft provides Comparer as a base class for implementations of IComparer<T>)

It doesn't make sense to compare instances of two different classes that each inherit from the base class. preventing that was the main reason for making the class generic (although it makes coding a derived class a little more complicate).

I would like to ask for a review of the code for the base class. Am I missing something? Can it be simplified? Is this a bad idea?

Here is the base class

public abstract class Comparable<T> : IComparable, IComparable<T>, IEquatable<T> where T: Comparable<T> {
 public abstract override int GetHashCode();
 public abstract int CompareTo(T other);
 public int CompareTo(object obj) {
 T other = obj as T;
 if (other == null && obj != null) {
 throw new ArgumentException($"Objects of type {typeof(T).Name} can only be compared to objects of the same type", nameof(obj));
 }
 return CompareTo(other);
 }
 public override bool Equals(object obj) {
 return CompareTo(obj) == 0;
 }
 new public bool Equals(T other) {
 return CompareTo(other) == 0;
 }
 private static int Compare(Comparable<T> comp1, Comparable<T> comp2) {
 if (comp1 == null) {
 return ((comp2 == null) ? 0 : -1);
 }
 return comp1.CompareTo(comp2);
 }
 public static bool operator == (Comparable<T> comp1, Comparable<T> comp2) {
 return Compare(comp1, comp2) == 0;
 }
 public static bool operator != (Comparable<T> comp1, Comparable<T> comp2) {
 return Compare(comp1, comp2) != 0;
 }
 public static bool operator > (Comparable<T> comp1, Comparable<T> comp2) {
 return Compare(comp1, comp2) > 0;
 }
 public static bool operator < (Comparable<T> comp1, Comparable<T> comp2) {
 return Compare(comp1, comp2) < 0;
 }
 public static bool operator >= (Comparable<T> comp1, Comparable<T> comp2) {
 return Compare(comp1, comp2) >= 0;
 }
 public static bool operator <= (Comparable<T> comp1, Comparable<T> comp2) {
 return Compare(comp1, comp2) <= 0;
 }
}

Below is a minimal implementation of the base class.

public class SeasonCompare : Comparable<SeasonCompare> {
 public int Number {get; set;}
 public override int GetHashCode() {
 return Number;
 }
 public override int CompareTo(SeasonCompare other) {
 if (other == null) {
 return 1;
 }
 return Number.CompareTo(other.Number);
 }
}

Answer 1

It is also recommended that I implement ... the non-generic IComparable.

I don't see that recommendation in the current doc for IComparable. I would recommend against it: having the non-generic method turns compile-time errors into runtime errors, which are more expensive to find and fix.

---

 private static int Compare(Comparable<T> comp1, Comparable<T> comp2) {
 if (comp1 == null) {
 return ((comp2 == null) ? 0 : -1);
 }
 return comp1.CompareTo(comp2);
 }

For consistency, this requires that subclasses guarantee that CompareTo(null) returns a positive value, but that requirement isn't documented.

Perhaps a better solution would be:

 private static int Compare(Comparable<T> comp1, Comparable<T> comp2) {
 if (comp1 == null) {
 return comp2 == null ? 0 : 0.CompareTo(comp2.CompareTo(comp1));
 }
 return comp1.CompareTo(comp2);
 }

That way the only requirement is that CompareTo(null) be consistent.

There may be a more elegant way of inverting the sense of a comparison, but that's the easiest one I can think of which doesn't fail on corner cases.

• Blackwood: Thanks @petertaylor. I suppose I just assumed that CompareTo(null) should return a positive value. I'll take your advice and change the Compare method.
• Blackwood: On the subject of implementing IComparable, it is true that this is not suggested in the documentation. I got the recommendation from a Stack Overflow answer (that I now can't find) that argued for implementing IComparable in order to avoid problems when sorting an ArrayList containing objects of the class. If I don't implement IComparable, I get an InvalidOperationException when sorting an ArrayList containing objects of my class. I wouldn't use an ArrayList myself, but I suppose I should allow for people who would. Do you still recommend against implementing IComparable?
• Peter Taylor: @Blackwood, I see it as a tradeoff between a little pain for people who chose runtime checks over compile-time checks by not upgrading to Net Framework 2.0 vs a little pain for everyone else. As far as I'm concerned, let the ArrayList users call a Sort overload which takes an IComparer.
• Blackwood: That makes sense (and simplifies my code). Thanks.

Answer 2

Stack Overflow Exception

You did not test your code appropriately because it throws the StackOverflowException for

(x == y)

This is the rare case when this happens and is triggered by these two methods.

public static bool operator ==(Comparable<T> comp1, Comparable<T> comp2)
{
 return Compare(comp1, comp2) == 0;
}

This calls the Compare method which in turn calls comp1 == null which means that Compare is called... and so on...

private static int Compare(Comparable<T> comp1, Comparable<T> comp2)
{
 if (comp1 == null)
 {
 return ((comp2 == null) ? 0 : -1);
 }
 return comp1.CompareTo(comp2);
}

When implementing comparers or equalities you should always use object.ReferenceEquals for checking arguments against null and never == null.

---

You should also use standard names for parameters like x & y or left & right and not comp1 and comp2.

• Blackwood: How embarrassing. I had not tested the equality operator since I converted the code from VB.Net (where comp1 Is Nothing does not cause the error) to C#. I apologize.
• t3chb0t: @Blackwood yeah, VB.NET is full of surprises by doing things differently than the rest of the world ;-)