When should I implement GetHashCode on an immutable struct?

Question

I've read some questions here, related to GetHashCode correct implementation. What I didn't found is when should I implement this method.

In my specific case, I've build a simple immutable struct :

public struct MyStruct
{
    private readonly Guid m_X;
    private readonly string m_Y;
    private readonly string m_Z;

    public Guid string X
    {
        get { return m_X; }
    }

    public string Y
    {
        get { return m_Y; }
    }

    public string Z
    {
        get { return m_Z; }
    }

    public MyStruct(Guid x, string y, string z)
    {
        if (x == Guid.Empty) throw new ArgumentException("x cannot be equals to Guid.Empty", "x");
        if (string.IsNullOrEmpty(y)) throw new ArgumentException("y cannot be null or empty", "y");
        if (string.IsNullOrEmpty(Z)) throw new ArgumentException("Z cannot be null or empty", "Z");

        this.m_X = x;
        this.m_Y = y;
        this.m_Z = Z;
    }

    public override int GetHashCode()
    {
        var x = 17;

        x = x * 23 + m_X.GetHashCode();
        x = x * 23 + m_Y.GetHashCode();
        x = x * 23 + m_Z.GetHashCode();

        return x;
    }
}

In this case, I've implemented GetHashCode, but was it mandatory? Isn't the base object.GetHashCode implementation itself handling this scenario?

[Edit] a bit of background: I have some string to parse and produces. This strings are part of a 3rd party custom query language. The string are always of the form X|Y|Z. I want to avoid developers to play with string.Split and string concatenation by providing this custom struct. Lastly, the struct will contains this two supplementary methods :

    public override string ToString()
    {
        return m_X.ToString() + "|" + m_Y + "|" + m_Z;
    }

    public static MyString Parse(string stringToParse)
    {
         // implementation omitted
    }

Don't use `&=` to combine the partial hashes. The `^23`s are completely useless. In short, your implementation of `GetHashCode` is *really* bad, barely better than `return 0`. — CodesInChaos, Nov 16 '12 at 14:18
Your new code is still bad. Use Jon Skeet's suggestion instead. — CodesInChaos, Nov 16 '12 at 14:21
The new code is fine as a simple implementation, IMO. See this, for example: http://stackoverflow.com/questions/263400/what-is-the-best-algorithm-for-an-overridden-system-object-gethashcode — Matthew Watson, Nov 16 '12 at 14:37
See my Hashcode builder helper class, which is based on Effective Java: http://stackoverflow.com/a/10634833/360211 — weston, Nov 16 '12 at 15:26

Jon Skeet · Answer 1 · 2012-11-16T14:21:47.597

8

Isn't the base object.GetHashCode implementation itself handling this scenario?

No, ValueType.GetHashCode() is actually handling it - and the default implementation does a really bad job, only using the first field in most cases. (In this case I'm not sure exactly what it would do given that your struct isn't "blittable" due to the string references. The equality can't be checked just from the simple binary values in the fields - the strings need to be checked for equality.)

For any value type you want to use for equality operations (e.g. as a key in a dictionary) it's a good idea to override GetHashCode and Equals(object), as well as implementing IEquatable<T> so that equality checking doesn't require boxing.

It's definitely a bad idea to override GetHashCode without overriding Equals - I'm surprised the compiler isn't warning you about that.

edited Nov 16 '12 at 14:21

answered Nov 16 '12 at 14:16

Jon Skeet

1,421,763
867
9,128
9,194

+1 Another good reason for dictionaries is that overriding this and `Equals` improves performance quite dramatically, as the default `struct` implementation is rather slow in this situation. – Adam Houldsworth Nov 16 '12 at 14:18
I've fix my implementation, to concentrate on *why/when*, and not *how* – Steve B Nov 16 '12 at 14:22
@SteveB: Hopefully the answer already covers that - and in particular, you should *definitely* be overriding `Equals(object)` and implementing `IEquatable` – Jon Skeet Nov 16 '12 at 14:25
Are you suggesting to implements this methods *every time* a custom struct is built? – Steve B Nov 16 '12 at 14:26
1

@SteveB: Yes, if you want to use it for equality checks. Just how often are you writing your own structs though? It's far more common to write classes - structs are rarely appropriate, IMO. – Jon Skeet Nov 16 '12 at 14:33
I've added the background as edit in my question. I'm open to any suggestion if appropriate – Steve B Nov 16 '12 at 14:35
1

@SteveB: That background doesn't provide any justification for it being a struct, nor any explanation as to why you haven't implemented an Equals method. – Jon Skeet Nov 16 '12 at 14:41
In fact,I see my struct as a composition of multiple base types. It's like the Point structure, composed by a X and Y sub values. If I pass a variable of this struct as a parameter, I would feel natural to have a copy of the value, not the original reference. I've not implemented the equals, because (at least before your answer), I was not aware that it was a mandatory (nearly) implementation. – Steve B Nov 16 '12 at 14:44
@SteveB The main reason for implementing `GetHashCode()` is for hashing performance reasons (such as key in dictionary) but it must always be in sync with `Equals()`! My belief is that you might gain from reading up on `GetHashCode()`, `Equals()` and `==` (the operator). See for example http://msdn.microsoft.com/en-us/library/7h9bszxx(v=vs.100).aspx . If you have the means, try http://www.amazon.com/Effective-Specific-Ways-Improve-Your/dp/0321245660 chapter 7, it is dedicated to `GetHashCode()` and helped me a lot when I went from "academic programming" to C#. – flindeberg Nov 16 '12 at 15:11
@SteveB If you have an immutable type, it makes far less sence for it to be a `struct` than a `class`. If a class, passing by reference is more efficient, and because no one can change it, there is no need to worry about the whole system pointing to just one copy in memory potentially. If a struct, it will be copied all over the place when passed by value, a waste of time and memory. The `Point` you refer to is OK as a struct because it is mutable. – weston Nov 16 '12 at 16:50
@weston: Absolutely not! Mutable structs should be avoided in almost all situations. While I tend to avoid creating structs anyway, when I *do* create structs they're pretty much *always* immutable. – Jon Skeet Nov 16 '12 at 16:57
That's the second "Absolutely not" I've got from you today! Doing well! I personally don't use structs either. I guess at least we agree there are few good reasons to use structs. – weston Nov 16 '12 at 18:56

score 0 · Answer 2 · answered Nov 16 '12 at 14:19

when should I implement this method

Considering that GetHashCode is used basically for comparison overriding, it has to be used when you have to implement your own comparison logic.

It could be a case, when, also, you're going to define similiarity of the not exactly equal (from the point of view of their content) instances of the struct. (can be defined like custom comparison logic too naturally)

It might to be created in the classes which are used like Keys inside dictionaries/hashes.

Classes that might be used as a key in a hash table must also override this method, because objects that are used as keys in a hash table are required to generate their own hash code through this method

When should I implement GetHashCode on an immutable struct?

2 Answers2