0

I override the Equals method for one of my class. In the method, I check the equality of each pair of a dictionary with those of the dictionary of another instance, like the following does

    public override bool Equals (object obj)
    {
        ...
        // compare to make sure all <key, value> pair of this.dict have
        // the match in obj.dict

        ...
    }

Now, I need to override the GetHashCode method as well as what is suggested.

Do I need to do that for all the keys of the dictionary, or keys plus values?

Basically, would the following be good or overkill?

public override int GetHashCode ()
{
    int iHash = 0;

    foreach (KeyValuePair<string, T> pair in this.dict)
    {
        iHash ^= pair.Key.GetHashCode();
        iHash ^= pair.Value.GetHashCode();
    }

    return iHash;
}
John Saunders
  • 160,644
  • 26
  • 247
  • 397
tom
  • 14,273
  • 19
  • 65
  • 124
  • 2
    http://stackoverflow.com/questions/371328/why-is-it-important-to-override-gethashcode-when-equals-method-is-overriden-in-c – Mitch Wheat Jul 30 '11 at 02:19
  • http://stackoverflow.com/questions/1378686/general-advice-and-guidelines-on-how-to-properly-override-object-gethashcode – Mitch Wheat Jul 30 '11 at 02:20
  • What are you trying to generate the hashcode for? Is this an custom class you've created? Or are you passing an IEqualityComparer for something to an existing dictionary? – Christopher Currens Jul 30 '11 at 02:38
  • Yes, this is in a custom class that I created. – tom Jul 30 '11 at 02:49

2 Answers2

2

Going along with what @Mitch Wheat linked to, that's not the best way to do a GetHashCode() if you use this class with a Dictionary or HashSet.

Imagine your internal Dictionary had only one entry. Your hash is now the value of that single KeyValuePair. You stick the whole class in a HashSet. You add another item to your internal Dictionary. Now the hashcode of your class has changed, because you're iterating over two items in your class.

When you call HashSet.Contains(obj), it calls obj.GetHashCode() which has now changed, even though its the same class instance. HashSet.Contains() will find that it doesn't contain this new hash and return false, never calling Equals (which will return true if the references are the same).

Suddenly its like your object has disappeared from the HashSet, even though the class is in there, with an outdated hash.

You really don't want your hash to change. It's okay to have collisions in your GetHashCode, because if it collides, it will call the (slower) .Equals() method. It's a handy optimization, that, if improperly implemented, can cause some headaches along the way.

As a side note, as pointed out in the link above, it's a good idea to multiply your hash by a prime number before ^ with another value. Helps with keeping the has unique.

Christopher Currens
  • 29,917
  • 5
  • 57
  • 77
  • 1
    that's a valid concern. what if the dictionary is immutable? would that be free of the problem that you indicated above? – tom Jul 30 '11 at 03:02
  • 1
    If the Key/Value pairs never change, and by that, I mean both no `KeyValuePairs` get added or removed, as well as the actual Keys and Values of each `KeyValuePair` don't get changed, then yes, you wouldn't have to worry about it. Keep in mind, inside of each `KeyValuePair`, the key or value can be a class, and that class can be mutable, and output a different HashCode if it is changed – Christopher Currens Jul 30 '11 at 03:06
0

Do you plan to use the object in a HashSet? You really only need to implement GetHashCode if the object is going to be used in such a way that it requires it be uniquely identifiable by its hash. It is good practice to always implement GetHashCode taking into account the same fields that are used in equality, but not always necessary.

If it is necessary in your case, I believe you have the right idea.

w.brian
  • 16,296
  • 14
  • 69
  • 118
  • Cool. That's what I want to know. It's purely for uniqueness. – tom Jul 30 '11 at 02:49
  • Glad I could help. If it is a satisfactory answer for you, it would be appreciated it you marked this as the answer. :) – w.brian Jul 30 '11 at 02:55