Должен ли я быть обеспокоен этой реализацией сравнения to / equals / hashCode? - PullRequest
3 голосов
/ 03 декабря 2010

Я нахожусь в процессе тестирования кода и нашел несколько случаев, когда у разработчика есть DTO, который реализует Comparable.Это DTO имеет 7 или 8 полей.Метод compareTo был реализован только для одного поля:

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

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

Аналогично, метод equals переопределяется и в основном сводится к:

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

и, наконец, реализация метода хеш-кода выглядит так:

return field1.hashCode;

field1 никогда не должно быть нулевым и будет уникальным для этих объектов (т.е. мы не должны получать два объекта с одинаковым field1).

Итак, реализации последовательны, что хорошо, но следует ли мне беспокоиться, что используется только одно поле?Это необычно?Это может вызвать проблемы или сбить с толку других разработчиков?Я имею в виду сценарий, когда список этих объектов передается, а другой разработчик использует карту или набор некоторого вида и получает необычное поведение от этих объектов.Любые мысли приветствуются.Спасибо!

Ответы [ 4 ]

5 голосов
/ 03 декабря 2010

Я подозреваю, что это случай "выигрышей при первом использовании" - кому-то нужно было отсортировать коллекцию этих объектов или поместить их в хэш-карту, и они заботились только о дате.Самым простым способом реализации этого было переопределить equals / hashCode и реализовать Comparable<T> так, как вы сказали.

Для специализированной сортировки лучшим подходом было бы внедрение Comparator<T> вдругой класс ... но к сожалению, в Java нет эквивалентного класса для проверки на равенство.Честно говоря, я считаю это серьезным недостатком в коллекциях Java.

Предполагая, что это действительно не"единственное естественное и очевидное сравнение", оно, безусловно, пахнет с точки зрения дизайна... и должен быть очень тщательно документ.

2 голосов
/ 03 декабря 2010

Вы не должны беспокоиться об этом, если field1 действительно уникально .Если это не так, у вас могут быть проблемы.Во всяком случае, я советую сделать несколько юнит-тестов.Они должны показать правду.

2 голосов
/ 03 декабря 2010

Строго говоря, это нарушает Спецификацию Comparable:

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

Обратите внимание, что null не является экземпляром какого-либо класса, и e.compareTo (null) должен выдать исключение NullPointerException, даже если e.equals (null) возвращает false.

Точно так же, похоже, что метод equals сгенерирует NPE на equals(null) вместо возврата false (если, конечно, вы "не выпарили" нулевой код обработки).

Это может вызвать проблемы или сбить с толку других разработчиков?

Возможно, возможно, нет. Это действительно зависит от того, насколько велик ваш проект и насколько широко будет использоваться / «многократно использоваться» / долговечен ваш исходный код объекта:

  • Малое / недолговечное / ограниченное использование == вероятно, не проблема.
  • Широкое / долгосрочное / широкое использование == нелогичная реализация может вызвать будущие проблемы
1 голос
/ 03 декабря 2010

Я не думаю, что вам нужно беспокоиться. Контракт между тремя методами сохраняется, и он соответствует.

Является ли это правильным с точки зрения бизнес-логики - это другой вопрос.

Если, например, field1 сопоставляется с первичным ключом в базе данных, это совершенно верно. Если field1 - это «имя» человека, я был бы обеспокоен

...