Search code examples
c#immutabilitymutablegethashcodeiequatable

Should GetHashCode be implemented for IEquatable<T> on mutable types?


I'm implementing IEquatable<T>, and I am having difficulty finding consensus on the GetHashCode override on a mutable class.

The following resources all provide an implementation where GetHashCode would return different values during the object's lifetime if the object changes:

However, this link states that GetHashCode should not be implemented for mutable types for the reason that it could cause undesirable behaviour if the object is part of a collection (and this has always been my understanding also).

Interestingly, the MSDN example implements the GetHashCode using only immutable properties which is in line with my understanding. But I'm confused as to why the other resources don't cover this. Are they simply wrong?

And if a type has no immutable properties at all, the compiler warns that GetHashCode is missing when I override Equals(object). In this case, should I implement it and just call base.GetHashCode() or just disable the compiler warning, or have I missed something and GetHashCode should always be overridden and implemented? In fact, if the advice is that GetHashCode should not be implemented for mutable types, why bother implementing for immutable types? Is it simply to reduce collisions compared to the default GetHashCode implementation, or does it actually add more tangible functionality?

To summarise my Question, my dilemma is that using GetHashCode on mutable objects means it can return different values during the lifetime of the object if properties on it change. But not using it means that the benefit of comparing objects that might be equivalent is lost because it will always return a unique value and thus collections will always fall back to using Equals for its operations.

Having typed this Question out, another Question popped up in the 'Similar Questions' box that seems to address the same topic. The answer there seems to be quite explicit in that only immutable properties should be used in a GetHashCode implementation. If there are none, then simply don't write one. Dictionary<TKey, TValue> will still function correctly albeit not at O(1) performance.


Solution

  • Mutable classes work quite bad with Dictionaries and other classes that relies on GetHashCode and Equals.

    In the scenario you are describing, with mutable object, I suggest one of the following:

    class ConstantHasCode: IEquatable<ConstantHasCode>
    {
        public int SomeVariable;
        public virtual Equals(ConstantHasCode other)
        {
            return other.SomeVariable == SomeVariable;
        }
    
        public override int GetHashCode()
        {
            return 0;
        }
    }
    

    or

    class ThrowHasCode: IEquatable<ThrowHasCode>
    {
        public int SomeVariable;
        public virtual Equals(ThrowHasCode other)
        {
            return other.SomeVariable == SomeVariable;
        }
    
        public override int GetHashCode()
        {
            throw new ApplicationException("this class does not support GetHashCode and should not be used as a key for a dictionary");
        }
    }
    

    With the first, Dictionary works (almost) as expected, with performance penalty in lookup and insertion: in both cases, Equals will be called for every element already in the dictionary until a comparison return true. You are actually reverting to performance of a List

    The second is a way to tell the programmers will use your class "no, you cannot use this within a dictionary". Unfortunately, as far as I know there is no method to detect it at compile time, but this will fail the first time the code adds an element to the dictionary, very likely quite early while developping, not the kind of bug happening only in production environment with an unpredicted set of input.

    Last but not least, ignore the "mutable" problem and implement GetHashCode using member variables: now you have to be aware that you are not free to modify the class when it's used withing a Dictionary. In some scenario this can be acceptable, in other it's not