0

When generating hashcodes for a class, is it ok to use the hashcodes of that class's members? Here is a sample class:

class Sample
{
    private readonly string _strA, _strB;
    public Sample(string strA, string strB)
    {
        this._strA = strA;
        this._strB = strB;
    }
    public override int GetHashCode()
    {
        return (this._strA + "###" + this._strB).GetHashCode();
    }
}

I think this will work as long as neither _strA nor _strB contain the string "###". I'm not totally sure though as I don't know the specifics of how hashcodes are generated on strings.

I saw a solution in the post at Create a hashcode of two numbers that I could tailor for my purposes, but I think that my solution is more simple (as long as neither string contains "###").

Community
  • 1
  • 1
user1214135
  • 625
  • 2
  • 11
  • 22
  • Seems to be a duplicate of : http://stackoverflow.com/questions/263400/what-is-the-best-algorithm-for-an-overridden-system-object-gethashcode – George Mamaladze Jul 31 '12 at 15:25
  • I don't see how. I offered a solution not mentioned in that post. I'd simply like to know if my solution is a good one. I haven't seen a solution like this mentioned in your post or any post in my search results. – user1214135 Jul 31 '12 at 15:33
  • As I understand you know what the best practice is, but anyway you are implementing something completely different from best practice and asking community wether it's ok or not? – George Mamaladze Jul 31 '12 at 15:47
  • 1
    If my potential solution is a duplicate of some other potential solution posted somewhere, please find that. Then we could call my question a duplicate. – user1214135 Jul 31 '12 at 16:29
  • Great argumentation. No further discussion needed. – George Mamaladze Jul 31 '12 at 16:35

2 Answers2

2

If you have several fields that contribute to the overall hash code for an object, a simple and fairly effective approach is this:

public override int GetHashCode()
{
    int hash = 17;

    hash = hash*23 + field1.GetHashCode();
    hash = hash*23 + field2.GetHashCode();
    hash = hash*23 + field3.GetHashCode();

    // And so on for all applicable fields.
    // field1, field2 and field3 are all class field members.

    return hash;
}
Matthew Watson
  • 104,400
  • 10
  • 158
  • 276
1

A better approach would be to combine the hash codes mathematically, using something like the Times 33 hash. In your current code you create a temporary string each time GetHashCode is called, which could suffer from poor performance.

public override int GetHashCode()
{
    // omit null-coalesce if we know them to be non-null
    return (33 * (this._strA ?? "").GetHashCode())
         + (this._strB ?? "").GetHashCode();
}

If your class is truly immutable, calculation of the hashcode up front may be worth the 4-bytes:

private readonly int _hash;

public Sample(string strA, string strB)
{
    this._strA = strA;
    this._strB = strB;
    this._hash = (33 * (this._strA ?? "").GetHashCode())
               + (this._strB ?? "").GetHashCode();
}

public override int GetHashCode()
{
    return this._hash;
}
Community
  • 1
  • 1
user7116
  • 63,008
  • 17
  • 141
  • 172
  • As in: it is idiomatic. *(ignoring the performance implications of creating a temporary string to create a hash code)* – user7116 Jul 31 '12 at 15:24
  • 2
    Adding (or XOR-ing) the component hash codes together is more general (works on more than strings). Also, catenating strings is relatively expensive, 'cos it allocates more strings, and should probably be avoided in GetHashCode. – Roger Lipscombe Jul 31 '12 at 15:24
  • 1
    What is it that makes this solution better? Does it run faster (maybe because you're not concatenating strings)? (I should have checked for nulls in my sample) Does it exhibit fewer collisions? I think that my solution exhibits the same number of collisions as .Net's string.GetHashCode() method. – user1214135 Jul 31 '12 at 15:28
  • I would imagine the number of collisions relies on your input domain. The distribution of hash codes for ASCII/English input is well known with the Bernstein hash. – user7116 Jul 31 '12 at 15:31