miércoles, 24 de octubre de 2012

Comparadores, igualdad y error difícil de detectar

Hola,

Después de leer por los internetes la enésima entrada sobre comparar objetos en Java y encontrar de nuevo el error que tienen el 90% de ellas, voy a aportar mi granito de arena para que quede constancia y alguno no se pille los dedos.

Para los que se aburren de leer pronto: Mucho ojo al devolver 0 en el método compare(Object o1, Object o2), tanto al implementar la interfaz Comparable como al crear un objeto Comparator.

La documentación dice que devolver 0 significa que los objetos son iguales, pero eso no es lo mismo que decir que nos da igual como estén ordenados, que eso es lo que entiende mucha gente instintivamente. Así que la pregunta es... ¿que ocurre cuando tenemos una Collection que no permite objetos repetidos ordenada por ese criterio?
La demostración, aquí:
import java.util.Arrays;
import java.util.Comparator;
import java.util.Set;
import java.util.TreeSet;

import lombok.Data;

public class App
{
  @Data
  static class ObjetoOrdenado{
    private final String nombre;
    private final Integer edad;    
  }
  
  static Comparator<objetoordenado> COMPARADOR_POR_EDAD =
     new Comparator<objetoordenado>()
  {
    @Override
    public int compare(ObjetoOrdenado o1, ObjetoOrdenado o2)
    {
      // Haciendo esto tendremos un problema, ya que
      // si devolvemos 0, uno de los elementos desaparecerá
      return o1.getEdad().compareTo(o2.getEdad());
    }
  };
  
  public static void main(String[] args)
  {
    ObjetoOrdenado[] aOrdenar = new ObjetoOrdenado[]{
        new ObjetoOrdenado("John",24)
        ,new ObjetoOrdenado("Jane",29)
        ,new ObjetoOrdenado("Bob",30)
        ,new ObjetoOrdenado("Susan",27)
        ,new ObjetoOrdenado("Kenny",24)
    };
    Set<objetoordenado> ordenados = new TreeSet(COMPARADOR_POR_EDAD);    
    ordenados.addAll(Arrays.asList(aOrdenar));
    for (ObjetoOrdenado o : ordenados)
    {
      System.err.println(o.getEdad() + " - " + o.getNombre());
    }
    System.err.println("They killed Kenny!");
  }
}
->

24 - John
27 - Susan
29 - Jane
30 - Bob


El resultado es que el pobre Kenny desaparece, eliminado por ser considerado "igual" que John al ordenar. La solución es no devolver 0 excepto cuando estemos seguros de que efectivamente son iguales, y si no pues devolvemos un 1 o un -1, lo que sea, pero distinto de 0.

Ésta implementación de compare sería una solución
import java.util.Arrays;
...
    @Override
    public int compare(ObjetoOrdenado o1, ObjetoOrdenado o2)
    {
      // Primero ordenamos por edad
      int temp = o1.getEdad().compareTo(o2.getEdad());
      if(temp==0)
      {
        // Si la edad es la misma, ordenamos por nombre
        temp = o1.getNombre().compareTo(o2.getNombre());
        // Si la edad y el nombre son iguales, ordenamos al azar.
        // Si realmente nombre y edad iguales significa que solo
        // queremos UN objeto, entonces podríamos saltar este paso y devolver 0
        if(temp==0)
        {
         temp = -1;  
        }
      }
      return temp;
    }
  };
...

Es un error difícil de detectar por que podemos tener una colección con todos los objetos, todo va bien, los ordenamos y ¡zas! mágicamente tenemos menos, sin excepciones, sin avisos.

Así que espero que a alguien le sirva para no caer en ello, siguiendo los típicos ejemplos de Internet que suelen explicarlo mal. Por dejarlo claro, yo caí en ello y por eso lo tengo bien aprendido :D.

Happy coding! EJ

3 comentarios:

  1. No se que carallo le pasa al Syntax Highlighter que ahora añade líneas extra al final del código. Le echaré un ojo a ver si lo puedo arreglar. :(

    ResponderEliminar
  2. Yo diría que el problema es más genérico. Se trata de definir correctamente equals/hashCode. En tu caso, el set no permite objetos repetidos y, como has definido un Comparator, utiliza el mismo para la igualdad.

    La solución: añades un test y listo.

    ResponderEliminar
  3. Hola xavi,
    No entiendo a que te refieres. Aunque implementes correctamente equals/hashCode siguen desapareciendo elementos, de hecho en el ejemplo ya están implementados correctamente gracias a Lombok y @Data.

    Y precisamente lo que menciones es, efectivamente, el problema que pretendo ilustrar, que se usa el Comparator no solo para el orden sino tambien para la igualdad y eso es lo que da problemas.

    No entiendo lo de que la solución es añadir un test. En la segunda implementación ya se hace "un test" para no devolver 0, si te refieres a eso. Supongo que no te referirás a hacer test unitarios, puesto que esos sirven para detectar problemas en el código, no para solucionarlos.

    ResponderEliminar