Checker CompareTo

belongs to group Basic
Identify incorrect definitions of compareTo()

Frameworks supported by this checker

  • java up to 11
  • android up to API level 28
  • dotnet

Warnings generated by this checker

  • AsymmetricalCompareToWarning: compareTo() is not symmetrical [ CWE596 ]
  • CompareToForNonObjectWarning: compareTo() is defined for a non-java.lang.Object argument but the comparable class is raw [ CWE227 ]
  • CompareToInNonComparableWarning: compareTo() is defined in a class that is not an instance of java.lang.Comparable [ CWE227 ]
  • CompareToInconsistentWithEqualsWarning: compareTo() is defined inconsistently from equals() [ CWE596 ]
  • CompareToWithDefaultEqualsWarning: compareTo() is defined but equals() is inherited from Object [ CWE596 ]

Options accepted by this checker

  • none

Annotations understood by this checker

  • none


Description

A class C that implements the raw interface java.lang.Comparable must implement the compareTo(Object) method. If C is made to implement the non-raw interface java.lang.Comparable<C>, then it must implement the compareTo(C) method instead.

Moreover, it is good practice to make compareTo() consistent with equals(): if the comparison of two objects yields 0, then they should be equal. The validity of this implication is in general undecidable. There are, however, frequent situations when, typically, this implication does not hold. An example is when equals() is inherited from java.lang.Object.

This checker verifies that compareTo(Object) is defined for classes implementing the raw java.lang.Comparable interface, instead of the (possibly more logical) method compareTo(C). Moreover, it verifies the consistency of compareTo() wrt equals().

Inconsistent definitions of compareTo()/equals() induce unexpected behaviors when objects are put inside most sorted collection classes of the standard Java library.

Action: use the non-raw interface java.lang.Comparable<C>; make equals() consistent with compareTo(). In most cases, this just amounts to providing the missing definition of equals().

A class C that implements the raw interface System.IComparable must implement the CompareTo(Object) method. If C is made to implement the non-raw interface System.IComparable<C>, then it must implement the CompareTo(C) method instead.

Moreover, it is good practice to make CompareTo() consistent with Equals(): if the comparison of two objects yields 0, then they should be equal. The validity of this implication is in general undecidable. There are, however, frequent situations when, typically, this implication does not hold. An example is when Equals() is inherited from System.Object.

This checker verifies that CompareTo(Object) is defined for classes implementing the raw System.IComparable interface, instead of the (possibly more logical) method CompareTo(C). Moreover, it verifies the consistency of CompareTo() wrt Equals().

Inconsistent definitions of CompareTo()/Equals() induce unexpected behaviors when objects are put inside most sorted collection classes of the standard .NET library.

Action: use the non-raw interface System.IComparable<C>; make Equals() consistent with CompareTo(). In most cases, this just amounts to providing the missing definition of Equals().

Examples

Consider the following program:

public abstract class CompareTo implements Comparable {
  private static int nextId;
  private final int id = nextId++;

  public final int compareTo(CompareTo other) {
    return id - other.id;
  }
}

This checker issues the following warning:

CompareTo.java:6: [CompareTo: CompareToForNonObjectWarning] Did you want to define compareTo(Object) here?

since the wrong compareTo signature is used. In this example, the programmer should make CompareTo implement Comparable<CompareTo> or change the signature of the method into compareTo(Object) and check the type of the actual parameter inside the method's body.

Consider now the following program:

public abstract class CompareTo implements Comparable<CompareTo> {
  private static int nextId;
  private final int id = nextId++;

  public final int compareTo(CompareTo other) {
    return id - other.id;
  }
}

class A extends CompareTo {
  private final String name;

  public A(String name) {
    this.name = name;
  }
}

This checker issues the following warning:

CompareTo.java:1: [CompareTo: CompareToWithDefaultEqualsWarning] compareTo(Object) seems inconsistent with equals(Object)

since there might be non-equal instances of class A whose comparison yields 0. In this example, the programmer should, for instance, modify class CompareTo as follows:

public abstract class CompareTo implements Comparable<CompareTo> {
  private static int nextId;
  private final int id = nextId++;

  @Override
  public final int compareTo(CompareTo other) {
    return id - other.id;
  }

  @Override
  public boolean equals(Object other) {
    return other instanceof CompareTo && ((CompareTo) other).id == id;
  }
}

Consider now the following program:

public final class CompareToVsEquals3 implements Comparable<CompareToVsEquals3> {
  private int f;
	
  public CompareToVsEquals3(int f) {
    this.f = f;
  }

  @Override
  public int compareTo(CompareToVsEquals3 o) {
    if (f > 0)
      return -1;
    else
      return 0;
  }

  @Override
  public String toString() {
    return String.valueOf(f);
  }
}

This checker issues the following warning:

CompareToVsEquals3.java:10: [CompareTo: AsymmetricalCompareToWarning] compareTo does not seem symmetrical: o1.compareTo(o2) != o2.compareTo(o1)
CompareToVsEquals3.java:10: [CompareTo: CompareToWithDefaultEqualsWarning] compareTo seems inconsistent with the inherited Object.equals

since this implementation of compareTo() is not symmetrical and might return 0 also when the inherited equals() method returns false. The programmer should correct this class for instance in the subsequent way:

public final class CompareToVsEquals3 implements Comparable<CompareToVsEquals3> {
  private int f;
	
  public CompareToVsEquals3(int f) {
    this.f = f;
  }

  @Override
  public int compareTo(CompareToVsEquals3 o) {
    return f - o.f;
  }

  @Override
  public boolean equals(Object other) {
    return other instanceof CompareToVsEquals3 && ((CompareToVsEquals3) other).f == f;
  }

  @Override
  public int hashCode() {
    return f;
  }

  @Override
  public String toString() {
    return String.valueOf(f);
  }
}

As another example, it might be the case that compareTo() and equals() are not consistent, as in the following class:

public final class CompareToVsEquals9 implements Comparable<CompareToVsEquals9> {
  private int f;
	
  public CompareToVsEquals9(int f) {
    this.f = f;
  }

  @Override
  public int compareTo(CompareToVsEquals9 o) {
    if (this == o || this.equals(o))
      return 1;
    else
      return -1;
  }

  @Override
  public boolean equals(Object other) {
    return other instanceof CompareToVsEquals9 && ((CompareToVsEquals9) other).f == f;
  }

  @Override
  public String toString() {
    return String.valueOf(f);
  }
}

This checker issues the following warning:

CompareToVsEquals9.java:10: [CompareTo: CompareToInconsistentWithEqualsWarning] compareTo does not seem to return 0 if and only if equals returns true

since when this.equals(o) we have this.compareTo(o) == 1, rather than this.compareTo(o) == 0, as required by the contract of these two methods. The same solution as for CompareToVsEquals3 above applies to this case.

Consider the following program:

using System;

namespace DocumentationExamples
{

    public abstract class CompareTo : IComparable
    {
        private static int nextId;
        private readonly int id = nextId++;
        int IComparable.CompareTo(CompareTo other)
        {
            return id - other.id;
        }
    }
    public class CompareToA : CompareTo
    {
        public static void Main(string[] args)
        { }

        private readonly string name;
        public CompareToA(string name)
        {
            this.name = name;
        }
    }
    public sealed class CompareToVsEquals : IComparable
    {
        private readonly int f;
        public CompareToVsEquals(int f)
        {
            this.f = f;
        }
        int IComparable.CompareTo(CompareToVsEquals o)
        {
            if (f > 0)
                return -1;
            else
                return 0;
        }
        public override string ToString()
        {
            return f.ToString();
        }
    }
    public sealed class CompareToVsEquals2 : IComparable
    {
        private int f;
        public CompareToVsEquals2(int f)
        {
            this.f = f;
        }
        int IComparable.CompareTo(CompareToVsEquals2 o)
        {
            if (this == o || Equals(o))
                return 1;
            else
                return -1;
        }
        public override bool Equals(object other)
        {
            return other is CompareToVsEquals2 && ((CompareToVsEquals2)other).f == f;
        }
        public override string ToString()
        {
            return f.ToString();
        }
        public override int GetHashCode()
        {
            return f.GetHashCode();
        }
    }
}

This checker issues the following warning:

DocumentationExamples.cs:11: [CompareTo: CompareToWithDefaultEqualsWarning] Method "CompareTo" seems inconsistent with method "equals" inherited from "Object" DocumentationExamples.cs:34: [CompareTo: CompareToWithDefaultEqualsWarning] Method "CompareTo" seems inconsistent with method "equals" inherited from "Object" DocumentationExamples.cs:34: [CompareTo: AsymmetricalCompareToWarning] Method "CompareTo" does not seem symmetrical DocumentationExamples.cs:53: [CompareTo: CompareToInconsistentWithEqualsWarning] Method "CompareTo" does not seem to return 0 if and only if "equals" returns true

since the wrong CompareTo signature is used. In this example, the programmer should make CompareTo implement IComparable or change the signature of the method into CompareTo(Object) and check the type of the actual parameter inside the method's body.