Overriding GetHashCode()

Question

In this article, Jon Skeet mentioned that he usually uses this kind of algorithm for overriding GetHashCode().

public override int GetHashCode()
{
  unchecked // Overflow is fine, just wrap
  {
    int hash = 17;
    // Suitable nullity checks etc, of course :)
    hash = hash * 23 + Id.GetHashCode();
    return hash;
  }
}

Now, I've tried using this, but Resharper tells me that the method GetHashCode() should be hashing using only read-only fields (it compiles fine, though). What would be a good practice, because right now I can't really have my fields to be read-only?

I tried generating this method by Resharper, here's the result.

public override int GetHashCode()
{
  return base.GetHashCode();
}

This doesn't contribute much, to be honest...

Answer 1

If all your fields are mutable and you have to implement GetHashCode method, I am afraid this is the implementation you would need to have.

public override int GetHashCode() 
{ 
    return 1; 
} 

Yes, this is inefficient but this is at least correct.

The problem is that GetHashCode is being used by Dictionary and HashSet collections to place each item in a bucket. If hashcode is calculated based on some mutable fields and the fields are really changed after the object is placed into the HashSet or Dictionary, the object can no longer be found from the HashSet or Dictionary.

Note that with all the objects returning the same HashCode 1, this basically means all the objects are being put in the same bucket in the HashSet or Dictionary. So, there is always only one single bucket in the HashSet or Dictionary. When trying to lookup the object, it will do a equality check on each of the objects inside the only bucket. This is like doing a search in a linked list.

Somebody may argue that implementing the hashcode based on mutable fields can be fine if we can make sure fields are never changed after the objects added to HashCode or Dictionary collection. My personal view is that this is error-prone. Somebody taking over your code two years later might not be aware of this and breaks the code accidentally.