Некоторые тонкости GetHashCode

[@mayorovp]

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

PS даже если изначальная архитектура оказалась ужасной, всегда есть способ сделать все правильно.

employees.Remove(employee);
employee.Snils = "654321";
employees.Add(employee);
Console.WriteLine(employees.Contains(employee));

Хотя, конечно же, без инкапсуляции куда-нибудь такой кусок кода является костылем

[@f0bos]

Я преднамеренно использовал HashSet, вопрос ключей здесь не стоял.
Цель статьи всего лишь показать грабли на которые можно наступить.

[@mholub]

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

В Java, C++ и еще паре десятков языков примерно та же реализация Hashset/Hashmap и ровно та же проблема.

[@lam0x86]

Полезная статья для новичков, но я бы хотел обратить внимание, что первопричина подобных ошибок — незнание внутренней архитектуры коллекций. Может, я сноб, но если человек не знает, как устроен Dictionary, я автоматом записываю его в джуниоры. Настольным инструментом любого .net-программиста должен быть декомпилятор.

[@lair]

На самом деле, ошибка тут даже не в реализации GetHashCode — а в том, что для «бизнес-задачи» (уж какая она может быть в примере) вы использовали коллекцию, не определив ее поведение.

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

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

Это не отменяет того, что правила по реализации GetHashCode должны включать в себя неизменность, просто пример далеко не самый удачный. Проблема скорее в том, как трактовать липпертовское «equal items have equal hashes» (проще говоря, зачем вообще оверрайдить дефолтную реализацию GetHashCode).

[@easyman]

Люди, вы о чём? Уже который коммент… Автор как смог, так и придумал пример неправильно работающего кода.

[@lair]

Мы о том, что если так сложно придумать хороший пример неправильно работающего кода, значит, проблема может быть где-то в другом месте.

[@f0bos]

Не-не, вы меня как-то не так поняли. В заголовке моей статьи не было фраз типа «80% продакшн кода написано некорректно» или чего-то похожего. Эту статью можно отнести к категории Tips & Tricks, или даже как пример вопроса на собеседовании на предмет определения уровня понимания дотнета. Меня позабавил тот факт, что ответив на автомате сам себе на «вопрос» из книги, я ошибся, вот и решил поделиться «радостью».

[@lair]

Понимаете ли, в чем дело: на мой взгляд, все это исследование должно предваряться вопросом «когда и зачем вообще нужно оверрайдить GetHashCode», потому что это само по себе намного интереснее — и, заодно, обычно исключает описанные вами ситуации.

[@PsyHaSTe]

Оверрайдить его нужно всегда, особенно для структур, потому что стандартная реализация не очень.

[@lair]

Не очень для чего?

[@int19h]

Ой ли? Ну, со структурами понятно, но вот для классов — вы что, серьезно предлагаете для каждого класса переопределять GetHashCode и Equals (потому что они всегда переопределяются парами)? Зачем?

[@PsyHaSTe]

Больше похоже на перекладывание ответственности за кривую архитектуру на средства автогенерации кода

[@int19h]

GetHashCode всегда должен идти в паре с Equals. Соответственно, если вы переопределяете GetHashCode описанным образом, то таким же образом должен быть переопределен и Equals. А это, по сути, просто кривой способ сказать «объект должен быть неизменяем» (если он изменяем, то по логике вещей и Equals должен меняться в зависимости от изменений). Соответственно, начинать этот разговор от GetHashCode — очень странно.

[@DragonFire]

Если реализовать корректно gethashcode и equals на изменяемом состоянии, но проблемы останутся те же… Элемент может попасть в одну корзину, а позже будет искаться в другой…

[@shai_hulud]

Guidelines and rules for GetHashCode
blogs.msdn.com/b/ericlippert/archive/2011/02/28/guidelines-and-rules-for-gethashcode.aspx