Devrais-je m'inquiéter de cette implémentation compareTo/equals/hashCode?

Question

Je suis au milieu de QA'ing un tas de code et j'ai trouvé plusieurs cas où le développeur a un DTO qui implémente Comparable. Ce DTO contient 7 ou 8 champs. La méthode compareTo a été mis en œuvre sur un seul terrain:

private DateMidnight field1; //from Joda date/time library 

public int compareTo(SomeObject o) { 
    if (o == null) { 
     return -1; 
    } 
    return field1.compareTo(o.getField1()); 
} 

De même, la méthode equals est surchargée et se résume essentiellement à:

return field1.equals(o.getField1()); 

et enfin la mise en œuvre de la méthode hashcode est:

return field1.hashCode; 

field1 ne devrait jamais être nul et sera unique à travers ces objets (c.-à-d. Nous ne devrions pas avoir deux objets avec le même field1). Donc, les implémentations sont cohérentes ce qui est bon, mais devrais-je être préoccupé par le fait qu'un seul champ est utilisé? Est-ce inhabituel? Est-ce que cela risque de causer des problèmes ou de dérouter les autres développeurs? Je pense au scénario où une liste de ces objets est passée et un autre développeur utilise une carte ou un ensemble de somesort et obtient un comportement inhabituel de ces objets. Toutes les pensées ont apprécié. Merci!

Answer 1

Je suppose qu'il s'agit d'un cas de «première utilisation gagne» - quelqu'un devait trier une collection de ces objets ou les mettre dans une carte de hachage, et ils seulement pris en compte la date. Le moyen le plus simple d'implémenter cela était de surcharger equals/hashCode et de mettre en œuvre Comparable<T> comme vous l'avez dit. Pour le tri spécialisé, une meilleure approche consisterait à implémenter Comparator<T> dans une classe différente ... mais Java n'a malheureusement pas de classe équivalente pour les tests d'égalité. Je considère que c'est une faiblesse majeure dans les collections Java, pour être honnête. En supposant que cela soit vraiment n'est pas "la comparaison naturelle et évidente", il sent certainement en termes de conception ... et devrait être très soigneusement document.

Answer 2

Je ne pense pas que vous ayez besoin de vous inquiéter. Le contrat entre les trois méthodes est conservé et c'est cohérent.

Que ce soit correct d'un point de vue de la logique métier est une question différente.

Si par ex. champ1 correspond à une clé primaire dans la base de données, il est parfaitement valide. Si field1 est le « prenom » d'une personne, je serais inquiet

Answer 3

à proprement parler, cela viole la spécification Comparable:

http://download.oracle.com/javase/6/docs/api/java/lang/Comparable.html

Notez que nul n'est pas une instance d'une classe, et e.compareTo (null) devrait lancer une exception NullPointerException même si e.equals (null) renvoie false.

De même, il ressemble à la méthode equals va jeter NPE sur equals(null) au lieu de retourner false (à moins bien sûr que vous « fait bouillir » le code de gestion null).

Est-ce que cela risque de causer des problèmes ou de dérouter les autres développeurs?

Éventuellement, possiblement pas. Cela dépend vraiment de la taille de votre projet et comment le code répandu/source « réutilisable »/long vécu votre objet devrait être utilisé:

• Petit/courte durée/utilisation limitée == probablement pas un problème .
• Grande/longue durée/utilisation généralisée == mise en œuvre de contre-intuitif peut causer des problèmes futurs

Answer 4

Vous ne devriez pas être concerné par, si Field1 est vraiment unique, . Si ce n'est pas le cas, vous pourriez avoir des problèmes. Quoi qu'il en soit, mon conseil est de faire quelques tests unitaires. Ils devraient montrer la vérité.