21

It is said that when input parameter is null, compareTo() should throw a NullPointerException. However, I am implementing a class which needs to compare fields with the type of String. These fields need not to be mandatory. I wonder in this case,

1) What should I return when the input is null? Should any not-null strings lexicographically bigger or smaller than null?

and

2) If this is considered bad practice, is there any supporting arguments? Should I force the user to use empty strings instead? If using empty string, won't that confuse the case in which the field is not applicable and the case in which the field is empty? And if exception must be thrown, then except from warning the user in the manual, what else could/shall I do?

EDIT: I might not express myself clearly here, but in the program I am implementing, the strings that could be null are all fields or a class, which should not be null. In other words, the objects comparedTo() uses could not be null, just their private fields could be. So in this case, I believe if I implement compareTo() properly, it would not violate the transitive requirement since classes with null fields would be considered the same always. Am I right or am I interpreting this wrong?

Thank you all for the answers!

zw324
  • 26,764
  • 16
  • 85
  • 118

6 Answers6

24

From javadoc for Comparable

Note that null is not an instance of any class, and e.compareTo(null) should throw a NullPointerException even though e.equals(null) returns false.

Bala R
  • 107,317
  • 23
  • 199
  • 210
16

Yes, there is no problem allowing null for instance fields - just make sure its sorting order is defined. Most natural would be putting it either before or after all real strings, but you could do anything here, just do it consistently. (For example, you could sort null like "null".)

Here is an example implementation for a single member:

class Example implements Comparable<Example> {

   @Nullable
   private String member;

   // TODO: getter, setter, constructor, ...

   public int compareTo(Example that) {
      if(this.member == null)
         if(that.member == null)
            return 0; //equal
         else
            return -1; // null is before other strings
       else // this.member != null
         if(that.member == null)
            return 1;  // all other strings are after null
         else
            return this.member.compareTo(that.member);
   }
}

Please note that the specification of Comparable.compareTo() only has a constraint for o.compareTo(null) (which should behave just like - null.compareTo(o), i.e. throw a NullPointerException), but not about how null fields are handled (it doesn't mention fields at all, so a class could return whatever it wants, as long as the antisymmetry, reflexivity and transitivity is ensured).

Paŭlo Ebermann
  • 73,284
  • 20
  • 146
  • 210
  • Well, there's the problem that it disagrees with the specification ;-) – user207421 Jun 07 '11 at 01:48
  • 2
    @EJP: The specification of `compareTo` says nothing on how (or even whether) to compare instance fields of the objects. – Paŭlo Ebermann Jun 07 '11 at 10:48
  • The specification of compareTo() says 'e.compareTo(null) should throw a NullPointerException'. – user207421 Jun 09 '11 at 10:47
  • 2
    Yes, but in this question (at least after the edit before my answer) it is not about comparing an object with `null`, but about comparing an object with another object, one of which has a `null` field. The `compareTo` specification says nothing about this. – Paŭlo Ebermann Jun 09 '11 at 13:54
  • the question before and after editing is 'What should I return when the input is null?' The Javadoc provides the answer. Your suggestion doesn't comply. – user207421 Jun 13 '11 at 10:00
  • 2
    @EJP Paulo is correct here. The original question asks what to do when `foo1.compareTo(foo2)` finds that `foo2 != null` but `foo2.someField == null`. The specification only dictates behavior when `foo == null`. – skelly Feb 17 '14 at 19:06
  • One comment: This won't work if you need to handle Asc/Desc order. I'll provide a solution with Asc/Desc. – gene b. Nov 16 '20 at 16:48
7

It would be a bad practice to not throw an exception because it violates the transitive antisymmetric nature of compareTo.

From Comparable.compareTo documentation:

The implementor must ensure sgn(x.compareTo(y)) == -sgn(y.compareTo(x)) for all x and y. (This implies that x.compareTo(y) must throw an exception iff y.compareTo(x) throws an exception.)

The implementor must also ensure that the relation is transitive: (x.compareTo(y)>0 && y.compareTo(z)>0) implies x.compareTo(z)>0.

Finally, the implementor must ensure that x.compareTo(y)==0 implies that sgn(x.compareTo(z)) == sgn(y.compareTo(z)), for all z.

More importantly, it's a bad idea to use compareTo on your objects to compare them with strings, for the same reason: sign(obj.compareTo(str)) != -sign(str.compareTo(obj)). Implement a custom Comparator and do whatever you want in it.

ykaganovich
  • 14,736
  • 8
  • 59
  • 96
  • Thanks! However, in the program I am implementing, the strings that could be null are all parts or a class, which should not be null. So in this case, I believe if I implement compareTo() properly, it would not violate the transitive requirement since classes with null fields would be considered the same always. Am I right or am I interpreting this wrong? – zw324 Jun 07 '11 at 00:08
  • It's perfectly fine to compare two instances of a class which might or might not have some instance fields set. Just make sure that you implement it transitively. I.E. c1.compareTo(c2) == -c2.compareTo(c1) – ykaganovich Jun 07 '11 at 00:13
  • 1
    Just for terminology: The condition `c1.compareTo(c2) == -c2.compareTo(c1)` is called antisymmetry, not transitivity. Transitivity is mainly the second condition in your quote (and maybe also the third). – Paŭlo Ebermann Jun 07 '11 at 00:18
  • @Paulo, thanks, you are correct, and also the antisymmetry requirement is weaker than what I had stated. It only has to do with signs, not actual values (subject to the transitivity constraint): `sign(c.compareTo(c2)) == -sign(c2.compareTo(c1))`. I've adjusted the answer; left previous comment for context. – ykaganovich Jun 07 '11 at 00:25
4

You need to decide if null is greater-than or less-than to a non-null value. You can design compareTo to meets the needs of your class's natural ordering, thus it is not bad practice.

Steve Kuo
  • 61,876
  • 75
  • 195
  • 257
3

Because the documentation of compareTo states that it should throw a NullPointerException, you should follow those guidelines so your implementation is consistent with the interface documentation. This also handles the questions of whether or not non-null strings are lexicographically smaller or bigger than null.

You have a couple of options on how to handle this. If empty and not-applicable are different, then you should probably wrap the string field in your own field class. For example, say you could create a type of MyField that might have an isApplicable method, which indicates if the field is applicable to this case (or something similar). Or you could rethink your design and be sure that an empty string and N/A really are two different things. If they are, you do need a way to differentiate between the two.

Jeff Storey
  • 56,312
  • 72
  • 233
  • 406
0

In addition to Paulo Ebermann's accepted answer, if you need to handle ASC/DESC order, you would do this. (We're assuming that NULLs always come before Non-NULLs in normal ascending order, otherwise for descending.)

    final boolean sortAsc = false; // Suppose this TRUE/FALSE is for ASC/DESC
    
    objects.sort(new Comparator<Example>() {

        @Override
        public int compare(Example e1, Example e2) {

            if (e1.getMember() == null && e2.getMember() == null) {
                return 0; // Both NULLs are equal
            }
            else if (e1.getMember() == null && e2.getMember() != null) {
                return sortAsc ? -1 : 1; // NULLs should precede non-NULLs in ascending order, follow in descending order
            }
            else if (e1.getMember() != null && e2.getMember() == null) {
                return sortAsc ? 1 : -1; // Non-NULLs should follow NULLs in ascending order, precede in descending order
            } else {
                // Both non-NULLs
                return sortAsc ? e1.getMember().compareTo(e2.getMember()) 
                               : e2.getMember().compareTo(e1.getMember());
            }
        }
        
    });
gene b.
  • 10,512
  • 21
  • 115
  • 227
  • 1
    It might be easier to just invert the existing comparator instead of having this case distinction three times, using [Comparator.reversed()](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/Comparator.html#reversed()) or Comparator.reverseOrder (based on the natural order). – Paŭlo Ebermann Nov 18 '20 at 00:08