0

I saw this hash function - and it triggered some alarms:

public override int GetHashCode()
{
    var result = 0;
    unchecked {
        result = anIntId.GetHashCode();
        result *= 397 * (aString != null ? aString.GetHashCode() : 0);
    }
    return result;
}

my automated hash-code-smell-fixer-subroutine wants to rewrite it to read:

public override int GetHashCode()
{
    var result = 0;
    unchecked {
        result = anIntId.GetHashCode() * 397;
        result = (result * 397) ^ (aString != null ? aString.GetHashCode() : 0);
    }
    return result;
}

i can't remember where i learned this pattern, but it seems that some of it doesn't actually make sense:

  • the first multiplication by 397 looks like a waste of time
  • [redacted] {in brain minus coffee context, interpreted ^ to be exponentiation}

does this seem correct, or am i making some mistakes?

blueberryfields
  • 45,910
  • 28
  • 89
  • 168
  • 2
    Math.Pow - exponentiation, ^ - xor – Atomosk Jun 26 '14 at 15:23
  • You shouldn't be using either multiplication or an XOR or exponentiation. You should be using addition. – Servy Jun 26 '14 at 15:32
  • @Servy mind showing an addition based equivalent implementation as an answer? – blueberryfields Jun 26 '14 at 15:39
  • @blueberryfields You replace the `*` before the generated hash code with a `+`. You really need an answer to see how to do that? – Servy Jun 26 '14 at 16:14
  • @servy i thought something more complex than that. with a +, you're going to get way too many spurious collisions for my tastes – blueberryfields Jun 26 '14 at 18:28
  • 1
    @blueberryfields No, you're not, because of the multiplication with the constant value. That's the whole point of multiplying with a constant. When you *only* multiply everything all of the constants are pointless. When you alternate between addition and multiplication (even if every other value is a constant) it does a very good job of eliminating collisions. – Servy Jun 26 '14 at 18:30
  • Removed Resharper tag. Not my downvote btw. – Piers Myers Jun 27 '14 at 12:31

0 Answers0