Комментарии 42
TL;DR:
1. Если объекты будут всегда сравниваться в пределах одной сессии Хибернейта, можно equals и hashCode вообще не переопределять
2. Если есть natural id или id выставляется самим приложением, используем только его в equals и hashCode
3. Если id генерирует база, используем его в equals и возвращаем из hashCode некоторую константу. Да, это ухудшит производительность коллекций на хешах, но серьезно это начнет влиять с такого количества объектов, что узким местом скорее станет сам факт вытаскивания их из БД. Такова идея, конечно, в реальном мире все зависит от того, как часто к этим коллекциям обращаться
Никогда не возвращайте из hashCode константу. Это бессмысленно, поскольку не просто ухудшает производительность хеш-таблиц, но сводит их к массиву. Но если вам достаточно массива, почему бы не использовать ArrayList вместо хеш-таблицы?
Если нам нужна коллекция уникальных элементов, заменив HashSet на ArrayList мы переложим ответственность за поддержание уникальности на себя. Придётся каждый раз вызывать contains и все в этом духе, то есть писать лишний код. А в сете это все уже за нас написано, пусть по производительности он будет сведён к массиву в случае константного hashCode. Да и что делать с Map?
Проблема в том, что иногда hashCode просто не от чего считать, вот и приходится выбирать между плохим решением и решением ещё хуже. Сущность, где все поля мутабельны — как раз такой случай
Ваши хешмапы превратились в тримапы. logn это не очень много, но все равно легко можно написать код который думая что там o(1) внезапно начнет тормозить на пустом месте. Особенно на активном добавлении-удалении элементов.
И естесвенно это произойдет на проде в НГ или черную пятницу. Когда минута простоя стоит несколько миллионов. Хорошо если рублей.
И все равно так не делайте. Это выстрелит через непонятное время.
Вот если написать в hashCode не константу, то да, выстрелит. Не сразу, но обязательно будут плавающие баги.
Ваши хешмапы превратились в тримапы. logn это не очень много, но все равно легко можно написать код который думая что там o(1) внезапно начнет тормозить на пустом месте.
Для этого не надо использовать энтити как ключи в хешмапе. А когда используешь коллекции типа HashSet, надо помнить, что нельзя класть туда много элементов. Но вообще надо просто помнить, что надо делать связи OneToMany в которых много элементов
Вы прячете в деталь реализации неожиданное усложнение .get из мапы до o(n).
Пишем типовой поиск всех элементов из соседней коллекции в этой и вот он квадрат.
И уже не надо много. Уже тысячах на 10 элементов тормоза будут. Которые не ловятся ни тестами ни ручными тестировщикам ни даже нагрузочным тестированием.
А вот на проде они замечательно выстрелят. В момент максимальной нагрузки. И потратят максимально возможное количество денег бизнеса.
Не надо так софт проектировать. Это гарантирует проблемы. Надо подумать и сделать хорошо. Именно за это деньги платят.
Вы прячете в деталь реализации неожиданное усложнение .get из мапы до o(n).
Поэтому не надо делать энтити ключами мапы
Пишем типовой поиск всех элементов из соседней коллекции в этой и вот он квадрат.
По перечисленным выше причинам, так делать не надо.
И уже не надо много. Уже тысячах на 10 элементов тормоза будут.
Во-первых, по перечисленным выше причинам на надо делать Set из десяти тысяч энтити. Потому что это приведёт к проблемам.
Во-вторых, десять тысяч элементов на связи OneToMany это много независимо от коллекции, которая используется для их хранения. В таких случаях лучше делать только связь ManyToOne потом выбирать элементы отдельным запросом.
Не надо так софт проектировать.
Я тоже говорю об этом.
Не надо использовать энтити в качестве ключей хешмепа. Не надо использовать что-то кроме id в equals и hashCode. hashCode должен возвращать константу. equals должен вернуть false если хотя бы один id равен null. Это актуально для энтити в которых нет естественного ключа, а суррогатный ключ не известен сразу после вызова конструктора объекта.
Не надо делать OneToMany с большим количеством участников. Это актуально для всех энтити.
Правил ещё много, но эти относятся к теме напрямую.
Уточнение — чтобы получился TreeMap, нужен способ сравнения записей. А его нет, так что там никакой не логарифм выйдет, а линия.
А разве нельзя в таком случае создать статичные суррогатное поле, которое будет при создании объекта заполняться некоторым предопределенным значением, хоть GUID - например, и его использовать как базу для расчёта hashcode? Вроде генерация GUID не очень затратна по времени и почти исключает дубликаты.
Проще не создавать никакого поля и использовать equals и hashCode по умолчанию.
Проще не создавать никакого поля и использовать equals и hashCode по умолчанию.
Такая позиция существует, но тогда получается, что будут проблемы с пониманием, что две энтити у которых равен id относятся к одной и той же строке в БД
А она так и так будет если программист не следит какие сущности он получает и из каких источников.
Если переопределить equals, то проблем с определением быть не должно, даже если одна из сущностей ещё не находится в контексте.
Ну а если обе сущности находятся в контексте, и программист не знает какая из них верная — то проблемы будут независимо от того как вы определили equals.
Никогда не возвращайте из hashCode константу.
За исключением случая, когда речь идёт о hashCode в классе, помечнном аннотацией Entity.
Это бессмысленно,
В случае с Entity это крайне осмысленно, потому что equals должен быть завязан на Id и Entity у которых Id == null должны быть не равны друг другу, чтобы можно было сложить их в Set
поскольку не просто ухудшает производительность хеш-таблиц, но сводит их к массиву.
Не к массиву, а к дереву, но да, сводит. Поэтому Entity не надо делать ключами в HashMap. Что касается коллекций типа Set — когда речь идёт об Entity, там в любом случае нельзя держать много элементов, так что ничего страшного не случится.
Но если вам достаточно массива, почему бы не использовать ArrayList вместо хеш-таблицы?
Обычно так и делают. Но даже в этом случае надо сделать так, чтобы arrayList.contains работал корректно, когда ему отдают Entity с id равным null
В случае с Entity это крайне осмысленно, потому что equals должен быть завязан на Id
Зачем?
Чтобы когда кладёшь в Set энтити и там уже есть энтити с таким id, новая энтити вытеснила старую.
Ну и иногда удобно делать arrayList.contains
Чтобы когда кладёшь в Set энтити и там уже есть энтити с таким id, новая энтити вытеснила старую.
В таком случае вы завязываетесь на недокументированное поведение.
А ведь можно вместо Set<EntityType> использовать Map<KeyType, EntityType> и ни на какое недокументированное поведение не завязываться. И бонусом тут будет ускорение работы.
В таком случае вы завязываетесь на недокументированное поведение.
Мне кажется нет такого. Какое поведение из того на что я ссылаюсь не документировано?
Чтобы когда кладёшь в Set энтити и там уже есть энтити с таким id, новая энтити вытеснила старую.
Вот это и недокументировано. Это просто особенность реализации HashSet и TreeSet. Более того, это ещё и на баг похоже, потому что в документации на интерфейс написано следующее:
Adds the specified element to this set if it is not already present (optional operation).
More formally, adds the specified element e to this set if the set contains no element e2 such that (e==null? e2==null: e.equals(e2)).
If this set already contains the element, the call leaves the set unchanged and returns false. In combination with the restriction on constructors, this ensures that sets never contain duplicate elements.
Это просто особенность реализации HashSet и TreeSet.
Контракт на интерфейс Set гарантирует, что если сделать сначала remove, а потом add, то в коллекции окажется новый объект. Наверное надо было сразу написать про все операции целиком.
Ну, если remove делать — то да, но вы же изначально без remove предлагали
Прошу прощения за недостаточную детализацию. Если серьёзно писать на эту тему — комментария не хватит. И вот эту важную деталь, как и многие другие, я не упомянул. Использование коллекций с JPA вообще нетривиальная штука. Мало того, что в некторых случаях надо делать remove а потом add одного и того же объекта, так ещё и надо делать много странных движений для маппинга, если хочешь убрать дополнительные запросы. Плюс надо помнить, что в Hibernate зачастую используется на ArrayList или что-то в этому духе, а какая-нибудь другая структура. Когда я пытаюсь вкратце рассказать, постоянно что-то упускаю.
Как вариант, можно договориться никогда не пихать в HashMap несохранённые хоть раз сущности. Это звучит гораздо разумнее, чем превращать HashMap в ArrayList.
Правильно — вообще не трогать эти методы.
Вариантов-то всего три: сравнивать по ссылке, сравнивать по id, сравнивать по всем атрибутам. Почему плох третий — автор написал, теперь сравним первые два варианта.
Если у двух сущностей совпадают id — это вовсе не означает что они равны, это означает что их нельзя использовать совместно, поскольку они представляют несогласованное знание об одном и том же предмете. А если сущности с одним id никогда не встречаются — то равенство id и равенство ссылок эквивалентны, но равенство ссылок проще и лишено некоторых недостатков (таких как поведение при незаполненном id). Следовательно, id можно при сравнении не проверять.
Есть ещё четвёртый вариант - самому генерировать ключ для мапки в зависимости от требуемой бизнес логикой уникальности: FirstName + LastName, email и т.п. А если уникальность не требуется то и мапка не нужна
Да, такой вариант есть. Но он ничем не отличается от варианта с id.
id можно использовать как ключ мапки чтобы, например, убедиться, что в бд не уйдет апдейт двух рекордов с одинаковым id. А свой ключ когда требуется какая-нибудь особая уникальность для новых и/или существующих entity (id пустой в первом случае)
Но ведь для этого не нужно же перегружать equals и hashCode...
Да. Честно говоря не ясен их смысл у мутабельных объектов. Добавили в сет, потом поменяли?
Честно говоря не ясен их смысл у мутабельных объектов.
Конкретно в энтити — есть необходимость безопасно добавлять и в Set.
Добавили в сет, потом поменяли?
Да, добавили в сет пока id == null, потом записали в БД и id сгенерировались и при этом сет должен работать корректно.
Вариантов-то всего три: сравнивать по ссылке, сравнивать по id, сравнивать по всем атрибутам. Почему плох третий — автор написал, теперь сравним первые два варианта.
Правильно сравнивать по Id, но добавить условие, что если id ==null, то equals выдаст false. Ну и плюс вернуть константу в hashCode конечно
Lombok + JPA: Что может пойти не так?