We're building some Roslyn analyzers concerning GetHashCode()
, including an analyzer that implements it for you in a given class.
While researching the subject we've found that there are many considerations to take into account, mainly on what kind of members should be avoided in a GetHashCode()
implementation.
The main kind of member to avoid are mutable members (to avoid hashcode changes when used in a collection).
In general, for mutable reference types, you should override
GetHashCode
only if:
- You can compute the hash code from fields that are not mutable
Now the question that remains: what kind of members are we talking about then? The member has to be immutable but then again it should also be (ideally) different for each instance otherwise you'll just end up with the same hashcode for every object.
This leads me to conclude that the only acceptable members for a GetHashCode()
implementation are readonly immutable structs and readonly immutable classes Note: not const
. Likewise no static
fields since they would be the same across instances anyway.
The way this would be implemented in reality would be by using a
- Field:
readonly T myField
- Property:
T myProp { get; }
Since it would be very not feasible to detect if a class is immutable, we should probably eliminate that entirely aside from string
perhaps.
This leaves the list of applicable members for a GetHashCode()
implementation to:
- Readonly fields
- And getter-only properties
- That are immutable structs
- And aren't
static
- And aren't interfaces
- +
string
Does that seem correct or have I made a mistake in the thought process somewhere? It seems like we're suddenly left with a lot less fields than people typically use. I realize that there is a pragmatic aspect to this that just says "add all the fields and let the dev figure it out" but I try to avoid introducing hidden bugs in the analyzers.
GetHashCode
implementation, and let the dev manually add whatever else they might need - if anything. Or do you intend VSD to always generate a foul-proofGetHashCode
implementation for anything?GetHashCode()
? They would be in turn checked by the analyzers and so would be assumed to be immutable as well.GetHashCode()
but I can't check what subclasses do. For exampleclass B : A
whereB
is mutable can be placed in a field of typeA
. I can look atA
's implementation (assuming I even have those sources available -- ergo, they're user-defined) but the field might as well get filled with an object of typeB
with the bad implementation. Structs don't have this issue because they can't be subclassed -- I know exactly which type will go in the field..GetHashCode()
, are you also going to provide an.Equals()
?structs
and are definitely included (bullet 3). Regarding Equals: yes, that is taken care of already and there we just use every field -- the rules are much more relaxed since we don't have to take a hash miss into account.