Допустимо ли возвращать порядковый номер объекта в GetHashCode()?

Question

[ArXen42]

Доброго времени суток. Имеется вот такой код:

namespace Stardust.Core.World
{
	public class SpaceObject
	{
		public SpaceObject()
		{
			LastLeasedID++;
			ID = LastLeasedID;
		}

		/// <summary>
		/// Уникальный идентификатор этого объекта. Не может быть изменен после создания.
		/// </summary>
		public readonly Int32 ID;

		public override Boolean Equals(Object obj)
		{
			var o = obj as SpaceObject;
			if (o != null)
				return this.ID == o.ID;

			return false;
		}

		public Boolean Equals(SpaceObject other)
		{
			return other != null && this.ID == other.ID;
		}

		public override Int32 GetHashCode()
		{
			return ID;
		}

		public static Int32 LastLeasedID { get; private set; }
	}
}

Т.е. это объект, который всегда имеет уникальный идентификатор, он же порядковый номер. Вопрос: корректна ли в данном случае такая реализация GetHashCode (equals вроде логичен)? В частности, не возникнет ли проблем со словарем, в котором будет пара десятков/сотен таких объектов? Есть подозрение, что такое неравномерное распределение будет плохо для реализации хеширования в словаре и возникнет много коллизий, но не уверен.

Comments

[Алексей Кулаков]

на заметку. такая реализация уникальности будет работать только если объекты существуют внури одного запуска приложения. если же ваш объект куда-то сохраняется и неким образом существует вне вашего приложения, то такая генерация идентификатора будет плохой затеей. чем не угодил старый добрый Guid? его нет в mono?

[ArXen42]

За GUID спасибо, буду изучать.
А по поводу сохранения - просто еще не реализовал сериализацию этого дела. Делаю игру под юнити, без сетевого взаимодействия, т.ч. только простая (относительно) механика save/load, там ввести дополнительные способы задать id при десериализации не проблема (а какой-нибудь DataContractJsonSerializer вообще сам приватный id восстановит, но идея использовать объекты, распакованные им напрямую без использования конструктора мне кажется не самой хорошей, хотя возможно я и ошибаюсь).

Answer 1

[Андрей]

Методы GetHashCode и Equals нужно переопределять, если вы хотите, чтобы два различных экземпляра одного класса могли считаться одинаковыми. В вашем же случае экземпляр будет равен только самому себе, потому что Id всегда будут уникальными. Получается, вы переизобрели семантику сравнения по ссылке, которая и так работает по умолчанию, если не переопределять GetHashCode и Equals вообще.

Еще одна проблема возникнет, если вы будете использовать этот код в многопоточном окружении - без синхронизации доступа к LastLeasedID рано или поздно получите дубликаты.

Comments

[ArXen42]

Спасибо за ответ, видимо действительно велосипед выдумываю. Вообще, были мысли, что такая реализация GetHashCode будет немного быстрее + исключит совпадение хешей у разных объектов, но возможно я просто не вижу каких-то проблем, т.ч. оставлю дефолтную. Да и Equals получился медленее, чем ReferenceEquals, странно, что сразу не заметил.

Синхронизацию LastLeasedID учту, спасибо.

Answer 2

[Виталий Витренко]

Универсальное правило переопределения хеш кода от Joshua Bloch

Store some constant nonzero value, say 17, in an int variable called result.
Compute an int hashcode c for each field f that defines equals:

• If the field is a boolean, compute (f ? 1 : 0)
  
• If the field is a byte, char, short, int, compute (int) f
  
• If the field is a long, compute (int) (f ^ (f >>> 32))
  
• If the field is a float, compute Float.floatToIntBits(f)
  
• If the field is a double, compute Double.doubleToLongBits(f), then hash the resulting long as in above.
  
• If the field is an object reference and this class's equals method compares the field by recursively invoking equals, recursively invoke hashCode on the field. If the value of the field is null, return 0.
  
• If the field is an array, treat it as if each element is a separate field. If every element in an array field is significant, you can use one of the Arrays.hashCode methods added in release 1.5.
  
• Combine the hashcode c into result as follows: result = 31 * result + c;
  

Использовать только id - нормально, но его нужно умножать на некоторое число.

Comments

[Андрей]

Умножать нужно в том случае, если возможных состояний у объекта больше, чем int.MaxValue. Тогда значение хэша сдвигается и умножается, чтобы хотя бы минимизировать вероятность коллизий (когда два разных объекта будут считаться одинаковыми). В случае с последовательно выдаваемым ID умножение, наоборот, может увеличить эту вероятность - int быстрее переполнится.

[Виталий Витренко]

Объекты с хеш кодом 1, 2, 3 попадут в один и тот же бакет, а с хеш кодом 1 * 17, 2 * 17, 3 * 17 в разные.

Answer 3

[Константин Бердников]

Вы определенно изобретаете велосипед. В практически любой IDE есть функция автоматической генерации этих методов с возможностью выбора полей, которые будут учитываться.
Что касается данной реализации, то аналогичный пример есть в документации