Voici un nouveau quiz, pour bien finir le mois de mai avant d'entamer ce beau mois de juin.

La classe ColoredPoint ci-dessous pose un problème assez subtil. Lequel ?

public class Point {
    private int x;
    private int y;
 
    public Point(int x, int y) {
        this.x = x;
        this.y = y;
    }
 
    public boolean equals(Object o) {
        if (this == o) { return true; }
        if (!(o instanceof Point)) { return false; }
 
        final Point point = (Point) o;
        return (x == point.x && y == point.y);
    }
 
    public int hashCode() {
        int result = x;
        result = 31 * result + y;
        return result;
    }
}
 
public class ColoredPoint extends Point {
    private Color color;
 
    public ColoredPoint(int x, int y, Color color) {
        super(x, y);
        this.color = color;
    }
 
    public boolean equals(Object o) {
        if (this == o) { return true; }
        if (!(o instanceof ColoredPoint && super.equals(o))) {
            return false;
        }
 
        final ColoredPoint point = (ColoredPoint) o;
        return (color == null ? (point.color == null) : color.equals(point.color));
    }
 
    public int hashCode() {
        int result = super.hashCode();
        result = 31 * result + (color != null ? color.hashCode() : 0);
        return result;
    }
}

Je suis désolé de décevoir Alexis, mais il est toujours bon de revenir de temps en temps sur ses classiques (et désolé aussi que le code du quiz soit aussi proche de celui du livre de J. Bloch : n'ayant pas ce livre dans ma bibliothèque, je ne m’en suis rendu compte qu’après coup.)

En effet, comme un certain nombre d’entre vous l’avaient vu (mais pas tous, semble-t-il), le problème vient de l’implémentation de Point.equals(). Notez que c'est ColoredPoint.equals() qui pose un problème, mais que c'est Point.equals() qui le crée (par contre, Point.hashCode() et ColoredPoint.hashCode() ne posent pas de problème particulier).

Si l’on revient à sa spécification, qui en détaille le contrat, la méthode Object.equals() doit être symétrique : quelles que soient les références x et y non-null, x.equals(y) doit retourner true si et seulement si y.equals(x) retourne true. Ce qui, comme l’a bien montré Jcs dans son commentaire, n’est pas le cas ici dès lors que l’on compare un Point et un ColoredPoint de mêmes coordonnées.

Au-delà de son caractère un peu théorique, est-ce si grave en pratique de violer cette règle de la symétrie ? Réponse : oui.

Joint à ce billet, vous trouverez une petite classe Java (TestEqualsWithSubclass) qui met clairement en évidence le problème que pose un equals() non symétrique. Elle reprend les deux classes Point et ColoredPoint ci-dessus. Elle définit également une petite classe ArraySet toute bête qui n’est rien d’autre qu’un Set reposant sur un ArrayList (et dont l’implémentation, bien que courte, est correcte et suffisante !)

Et le main de cette classe montre que même si on ajoute les mêmes objets à deux implémentations de Set différentes, l’un des Set peut prétendre avoir un contenu différent de l’autre ! (La méthode contains(), appelée avec le même objet, pouvant retourner un résultat différent selon l’implémentation de Set choisie.)

Ce genre de bug est assez difficile à débusquer. Par exemple, dans la classe jointe à ce billet, la plupart des développeurs vont commencer par penser que le bug se trouve dans l’implémentation de Set, sans imaginer une seconde qu'il puisse en fait se trouver dans une des classes dont on a placé des instances dans ce Set. C’est d’autant plus gênant que le code produit par certains générateurs de code d’IDE contient cette erreur.

Dernière question : comment corriger cette erreur ? Réponse : dans le code du quiz, on peut remplacer chacun des deux tests "o instanceof Point" et "o instanceof ColoredPoint" par "this.getClass() == o.getClass()". (En fait, ici, ne remplacer que le test sur Point suffit : preuve que le problème est bien créé par la classe Point et non ColoredPoint. Néanmoins, on retrouverait le même problème avec ColoredPoint si on la sous-classait.)

La solution semble bonne, mais un autre problème survient alors : un ColoredPoint n’est plus tout à fait une sorte de Point, puisque Point est une classe concrète, et qu'on a garanti qu'un Point et un ColoredPoint ne pouvaient être égaux.

Autrement dit, on ne peut pas éviter le problème, à moins de violer un des principes fondamentaux de l’objet, l’héritage.