Pull to refresh

Comments 16

Не следует использовать в качестве ключей изменяемые структуры данных. А если пришлось — то не следует их после этого менять… GetHashCode тут ни при чем.

PS даже если изначальная архитектура оказалась ужасной, всегда есть способ сделать все правильно.
employees.Remove(employee);
employee.Snils = "654321";
employees.Add(employee);
Console.WriteLine(employees.Contains(employee));

Хотя, конечно же, без инкапсуляции куда-нибудь такой кусок кода является костылем
Я преднамеренно использовал HashSet, вопрос ключей здесь не стоял.
Цель статьи всего лишь показать грабли на которые можно наступить.
Грабли всего лишь состоят в том, что система типов C# не настолько мощная, чтобы указать требование о неизменяемости типа для ключа для Hashset.
Поэтому это требование приходится держать в голове.

В Java, C++ и еще паре десятков языков примерно та же реализация Hashset/Hashmap и ровно та же проблема.
Полезная статья для новичков, но я бы хотел обратить внимание, что первопричина подобных ошибок — незнание внутренней архитектуры коллекций. Может, я сноб, но если человек не знает, как устроен Dictionary, я автоматом записываю его в джуниоры. Настольным инструментом любого .net-программиста должен быть декомпилятор.
На самом деле, ошибка тут даже не в реализации GetHashCode — а в том, что для «бизнес-задачи» (уж какая она может быть в примере) вы использовали коллекцию, не определив ее поведение.

Дело в том, что вопрос «по каким признакам два человека являются одним» (иными словами, что уникально идентифицирует человека) — он открытый, и определяется задачей. Соответственно, и поведение вашего HashSet<Employee> должно зависеть от бизнеса. Может быть, в конкретном куске кода вам нужно, чтобы два человека с одним СНИЛС считались одним (и, соответственно, с разными — разным)? Тогда ваш код ведет себя правильно. А может быть, у вас тут вообще не бизнес-поведение, а какая-то странная глубоко внутренняя функция, и вам нужно, чтобы в хэшсете было по одному экзмепляру каждого объекта безотносительно их значений (т.е., равенство по ReferenceEquals)?

И вот чтобы никогда не возникало вопросов, что же именно вы имели в виду, используя HashSet, осмысленно при его создании явно указывать comparer. Тогда ваше намерение будет очевидно и читаемо.

Это не отменяет того, что правила по реализации GetHashCode должны включать в себя неизменность, просто пример далеко не самый удачный. Проблема скорее в том, как трактовать липпертовское «equal items have equal hashes» (проще говоря, зачем вообще оверрайдить дефолтную реализацию GetHashCode).
Люди, вы о чём? Уже который коммент… Автор как смог, так и придумал пример неправильно работающего кода.
Мы о том, что если так сложно придумать хороший пример неправильно работающего кода, значит, проблема может быть где-то в другом месте.
Не-не, вы меня как-то не так поняли. В заголовке моей статьи не было фраз типа «80% продакшн кода написано некорректно» или чего-то похожего. Эту статью можно отнести к категории Tips & Tricks, или даже как пример вопроса на собеседовании на предмет определения уровня понимания дотнета. Меня позабавил тот факт, что ответив на автомате сам себе на «вопрос» из книги, я ошибся, вот и решил поделиться «радостью».
Понимаете ли, в чем дело: на мой взгляд, все это исследование должно предваряться вопросом «когда и зачем вообще нужно оверрайдить GetHashCode», потому что это само по себе намного интереснее — и, заодно, обычно исключает описанные вами ситуации.
Оверрайдить его нужно всегда, особенно для структур, потому что стандартная реализация не очень.
Не очень для чего?
Ой ли? Ну, со структурами понятно, но вот для классов — вы что, серьезно предлагаете для каждого класса переопределять GetHashCode и Equals (потому что они всегда переопределяются парами)? Зачем?
Больше похоже на перекладывание ответственности за кривую архитектуру на средства автогенерации кода
GetHashCode всегда должен идти в паре с Equals. Соответственно, если вы переопределяете GetHashCode описанным образом, то таким же образом должен быть переопределен и Equals. А это, по сути, просто кривой способ сказать «объект должен быть неизменяем» (если он изменяем, то по логике вещей и Equals должен меняться в зависимости от изменений). Соответственно, начинать этот разговор от GetHashCode — очень странно.
Если реализовать корректно gethashcode и equals на изменяемом состоянии, но проблемы останутся те же… Элемент может попасть в одну корзину, а позже будет искаться в другой…
Sign up to leave a comment.

Articles