Improving Custom Comparator.compare() method

Question

I have the class SeguimientoSeccion with this relevant attributes

public class SeguimientoSeccion implements Serializable {
 private Integer id;
 private Transportista transportista;
 private String fijacion;
 private Integer month;
 private Integer year;
}

I created a custom comparator to sort by

1. year - Integer
2. month - Integer
3. fijacion - String
4. Transportista.nombre - String

SeguimientoSeccionComparator.java (class I want to improve)

public class SeguimientoSeccionComparator implements Comparator<SeguimientoSeccion> 
{
 public int compare(SeguimientoSeccion o1, SeguimientoSeccion o2) 
 {
 int result = o1.getYear().compareTo(o2.getYear());
 if (result == 0) {
 result = o1.getMonth().compareTo(o2.getMonth());
 }
 if (result != 0) return result;
 if (o1.getFijacion() == null && o2.getFijacion() == null)
 result = 0;
 else if (null == o1.getFijacion())
 result = 1;
 else if (null == o2.getFijacion())
 result = -1;
 else
 result = o1.getFijacion().compareTo(o2.getFijacion());
 if (result != 0) return result;
 if (o1.getFijacion() == null && o2.getFijacion() == null)
 result = 0;
 else if (null == o1.getTransportista())
 result = 1;
 else if (null == o2.getTransportista())
 result = -1;
 else
 result = o1.getTransportista().getNombre().compareTo(o2.getTransportista().getNombre());
 return result;
 }
}

EXTRA INFO:

equals and hashCode autogenerated by Eclipse IDE:

@Override
public int hashCode() {
 final int prime = 31;
 int result = 1;
 result = prime * result
 + ((fijacion == null) ? 0 : fijacion.hashCode());
 result = prime * result + ((id == null) ? 0 : id.hashCode());
 result = prime * result + ((month == null) ? 0 : month.hashCode());
 result = prime * result
 + ((transportista == null) ? 0 : transportista.hashCode());
 result = prime * result + ((year == null) ? 0 : year.hashCode());
 return result;
}
@Override
public boolean equals(Object obj) {
 if (this == obj)
 return true;
 if (obj == null)
 return false;
 if (getClass() != obj.getClass())
 return false;
 SeguimientoSeccion other = (SeguimientoSeccion) obj;
 if (fijacion == null) {
 if (other.fijacion != null)
 return false;
 } else if (!fijacion.equals(other.fijacion))
 return false;
 if (id == null) {
 if (other.id != null)
 return false;
 } else if (!id.equals(other.id))
 return false;
 if (month == null) {
 if (other.month != null)
 return false;
 } else if (!month.equals(other.month))
 return false;
 if (transportista == null) {
 if (other.transportista != null)
 return false;
 } else if (!transportista.equals(other.transportista))
 return false;
 if (year == null) {
 if (other.year != null)
 return false;
 } else if (!year.equals(other.year))
 return false;
 return true;
}

• Spotted: Are you on Java 8 ?
• Jordi Castilla: @Spotted unfortunatelly, nope, JDK1.7.0.51x64
• rolfl: And.... my answer is 54 seconds late...
• greybeard: (There was a C&P-error in the intended "both null"-condition before the getTransportista() invocations. The getNombre().compareTo() goes unchecked...)

Answer 1

You should consider using the native language/library support for work like this.

Let me prime that, by adding this private function to your clas:

private Integer getTransNombre() {
 return transportista == null ? null : transportista.getNombre();
}

That gets the number, if the transportista is not null. Now, using that function, we can create a reusable comparator instance using the tricks that Java 8 introduces. Consider these chained comparators:

private static final Comparator<SeguimientoSeccion> COMP = Comparator.comparing(SeguimientoSeccion::getYear)
 .thenComparing(SeguimientoSeccion::getMonth)
 .thenComparing(SeguimientoSeccion::getFijacion, Comparator.nullsLast(Comparator.naturalOrder()))
 .thenComparing(SeguimientoSeccion::getTransNombre, Comparator.nullsLast(Comparator.naturalOrder()));

How does that work? It gets details about the instances, if they compare as "equals" then the next test is run. You can sort nulls to the end, and you can go from there.

If you still need your comparator class, it could be:

public class SeguimientoSeccionComparator implements Comparator<SeguimientoSeccion> {
 public int compare(SeguimientoSeccion o1, SeguimientoSeccion o2) {
 return COMP.compare(o1, o2);
 }
}

Answer 2

Given

/** Note: this class has a natural ordering
 * that is inconsistent with equals. */
static class Transportista
 implements Comparable<Transportista> {
 String nombre;
 String getNombre() { return nombre; }
 @Override
 public int compareTo(Transportista o) { return 0; }
}
/** mock Java 8 nullsLast comparison */
static int nullsLast(Comparable c, Comparable x) {
 if (c == x)
 return 0;
 if (null == c)
 return 1;
 if (null == x)
 return -1;
 return c.compareTo(x); 
}

, compare() could look

/** by {@code year, month, fijacion, Transportista.nombre} */
@Override
public int compare(SeguimientoSeccion o1, SeguimientoSeccion o2) 
{
 int result;
 final Transportista t1, t2;
 if ( 0 != (result = o1.getYear().compareTo(o2.getYear()))
 || 0 != (result = o1.getMonth().compareTo(o2.getMonth()))
 || 0 != (result = nullsLast(o1.getFijacion(),
 o2.getFijacion()))
 || 0 != (result = nullsLast(t1 = o1.getTransportista(),
 t2 = o2.getTransportista())))
 return result;
 return nullsLast(t1.getNombre(), t2.getNombre());
}

(null handling of nombres differs from the question ever so slightly.)