How can I compare null values using Comparator?

Question

I've got a few Comparators -- one for Dates, one for decimals, one for percentages, etc.

At first my decimal comparator looked like this:

class NumericComparator implements Comparator<String> {

  @Override
  public int compare(String s1, String s2) {
    final Double i1 = Double.parseDouble(s1);
    final Double i2 = Double.parseDouble(s2);
    return i1.compareTo(i2);
  }

}

Life was simple. Of course, this doesn't handle the case where the strings aren't parseable. So I improved compare():

class NumericComparator implements Comparator<String> {

  @Override
  public int compare(String s1, String s2) {
    final Double i1;
    final Double i2;

    try {
      i1 = Double.parseDouble(s1);
    } catch (NumberFormatException e) {
      try {
        i2 = Double.parseDouble(s2);
      } catch (NumberFormatException e2) {
        return 0;
      }
      return -1;
    }
    try {
      i2 = Double.parseDouble(s2);
    } catch (NumberFormatException e) {
      return 1;
    }

    return i1.compareTo(i2);
  }
}

Life was better. Tests felt more solid. However, my code reviewer pointed out, "What about nulls?"

Great, so now I have to repeat the above with NullPointerException or prepend the method body with:

if (s1 == null) {
  if (s2 == null) {
    return 0;
  } else {
    return -1;
  }
} else if (s2 == null) {
  return 1;
}

This method is huge. The worst part is, I need to repeat this pattern with three other classes which compare different types of strings and could raise three other exceptions while parsing.

I'm not a Java expert. Is there a cleaner, neater solution than -- gasp -- copying and pasting? Should I trade correctness for lack of complexity so as long as it is documented?

---

Update: Some have suggested that it's not the Comparator's job to handle null values. Since the sort results are displayed to users I indeed want nulls to be sorted consistently.

Answer 1

You are implementing a Comparator<String>. String's methods, including compareTo throw a NullPointerException if a null is handed in to them, so you should too. Similarly, Comparator throws a ClassCastException if the arguments' types prevent them from being compared. I would recommend you implement these inherited behaviors.

class NumericComparator implements Comparator<String> {

  public int compare(String s1, String s2) {
    final Double i1;
    final Double i2;
    if(s1 == null)
    {
      throw new NullPointerException("s1 is null"); // String behavior
    }
    try {
      i1 = Double.parseDouble(s1)
    } catch (NumberFormatException e) {
      throw new ClassCastException("s1 incorrect format"); // Comparator  behavior
    }

    if(s2 == null)
    {
      throw new NullPointerException("s2 is null"); // String behavior
    }
    try {
      i2 = Double.parseDouble(s1)
    } catch (NumberFormatException e) {
      throw new ClassCastException("s2 incorrect format"); // Comparator  behavior
    }
    return i1.compareTo(i2);
  }
}

You can almost regain the original elegance by extracting a method to do the type checking and conversion.

class NumericComparator implements Comparator<String> {

  public int compare(String s1, String s2) {
    final Double i1;
    final Double i2;

    i1 = parseStringAsDouble(s1, "s1");
    i2 = parseStringAsDouble(s2, "s2");
    return i1.compareTo(i2);
  }

  private double parseStringAsDouble(String s, String name) {

    Double i;
    if(s == null) {
      throw new NullPointerException(name + " is null"); // String behavior
    }
    try {
      i = Double.parseDouble(s1)
    } catch (NumberFormatException e) {
      throw new ClassCastException(name + " incorrect format"); // Comparator  behavior
    }
    return i;
  }
}

If you are not particular about the Exception messages, you can lose the "name" parameter. I'm sure you can lose an extra line here or word there by applying little tricks.

You say you need to repeat this pattern with three other classes which compare different types of strings and could raise three other exceptions. It's difficult to offer specifics there without seeing the situation, but you may be able to use "Pull Up Method" on a version of my parseStringAsDouble into a common ancestor of NumericComparator that itself implements java's Comparator.