0

I got reading this question here which led me to this article here.

I've got an abstract base class that allows me to constrain methods to accept only my classes that extend my abstract base class (basic polymorphism). My question is: Can I implement GetHashCode() in my abstract base class to provide a suitable override for any concrete implementation? (I.e. to avoid overriding GetHashCode() in each concrete class.)

I'm imagining a method in my abstract base class something like this:

public abstract class FooBase
{
    private static readonly int prime_seed = 13;
    private static readonly int prime_factor = 7;

    public override int GetHashCode()
    {
        // Seed using the hashcode for this concrete class' Type so
        // two classes with the same properties return different hashes.
        int hash = prime_seed * this.GetType().GetHashCode();
        // Get this concrete class' public properties.
        var props = this.GetType().GetProperties(BindingFlags.Public);
        foreach (var prop in props)
        {
            // Factor in each of this concrete class' public properties' hashcodes.
            hash = (hash * prime_factor) + prop.GetHashCode();
        }
        return hash;
    }
}

This seems to work in some basic equality unit tests but I feel like I've overlooked something. I still have to provide an override in each concrete class to avoid the compiler Warning about not overriding GetHashCode(), but at least this way I don't have to manually write an implementation for each one.

Community
  • 1
  • 1
JMD
  • 7,331
  • 3
  • 29
  • 39
  • seems like it would be slow to involve reflection for hashing – Greg Dean Sep 26 '12 at 19:17
  • Following up, I went and did some more reading about the implementation and use of GetHashCode() and Equals() and their relationship to each other. After gaining a better understanding, I scrapped the base GetHashCode() I was working on, above. Queue one of those old NBC PSAs: The More You Know... – JMD Oct 04 '12 at 19:18
  • One of the reasons it's always good to override `GetHashCode()` in `structs` is precisely to avoid the default which uses the same general idea as this. It works most of the time, but it's a lot slower than most simple `GetHashCode()` overrides you can code in less than a minute. – Jon Hanna May 13 '15 at 10:10

3 Answers3

2

Does this perform better than:

public override int GetHashCode()
{
    return 1;
}

A key to a Hash function is it must be fast to compute. With reflection, you might lose all the benefits you stand to gain from having a hash.

It would be worth it to benchmark.

Also, hash codes must be equal when Equals returns true, so will all subclasses use only public properties in their Equals method? If not, you might consider looping over all properties, not just public ones.

Edit to add: Also, consider adding unchecked around your property loop to prevent an exception if the hash gets bigger than Int.MaxValue.

Jason
  • 1,385
  • 9
  • 11
  • +1 especially on "hash codes matching Equals" remark. Note that by itself reflection does not make GetHashCode slow as result can be cached if no changes made to the object. The problem is that ensuring object is not changed between calls is non-trivial. – Alexei Levenkov Sep 26 '12 at 19:30
  • Thanks for the chastisement about performance and the note about `unchecked`. In my case I won't be dealing with enough objects (perhaps a couple thousand instances) for the performance to be noticeably impacted. Still, I'm going to go back and reconsider the direction I was heading. – JMD Sep 26 '12 at 20:24
1

You missed a basic rule - GetHashCode() result must remain the same for the whole lifetime of the object. In your GetHashCode implementation, it isn't guaranteed (because the properties you are iterating can be mutable).

Shahar Gvirtz
  • 2,418
  • 1
  • 14
  • 17
  • 2
    -1 - this is not true. If you check out http://msdn.microsoft.com/en-us/library/system.object.gethashcode.aspx it explicitly says "must consistently return the same hash code as long as there is *no modification to the object state*" – Alexei Levenkov Sep 26 '12 at 19:26
  • 1
    Eric Lippert wrote: "Rule: the integer returned by GetHashCode must never change while the object is contained in a data structure that depends on the hash code remaining stable". He also wrote that the guideline is that the result will never change. for his complete explanation, read the original post: http://blogs.msdn.com/b/ericlippert/archive/2011/02/28/guidelines-and-rules-for-gethashcode.aspx – Shahar Gvirtz Sep 26 '12 at 19:36
  • It is not possible for the result of GetHashCode() to remain the same for the whole lifetime of the object if the hash is being derived from the object's state. That rule would invalidate every example of generating a hashcode using an object's state. By the rule, from the moment construction begins you would not be allowed to change any of the values that go into deriving the hashcode. – JMD Sep 26 '12 at 20:14
  • 2
    Not exactly, it just means that you should generate the hashcode based on immutable members... – Shahar Gvirtz Sep 26 '12 at 20:18
  • `GetHashCode()` **must** change if the object changes in such a way as to make it no longer equal to an object to which it was previously equal, or equal to an object to which it was not previously equal. If that causes problems (e.g. the object was being used as a key to a dictionary when it changed) the problem was with either the change or the fact that it's being used for non-reference equality in the first place, not the fact that the value returned by `GetHashCode()` changed. – Jon Hanna May 13 '15 at 10:08
0

Using reflection is rather slow, but in some cases it is acceptable. The GetHashCode function is used when the instance of the class is a key in a hashtable or dictionary. In most cases you know when you need it. There are some tools, like Resharper (example) which can generate the GetHashCode and Equals functions for you instead of writing them manually.

Andrei Schneider
  • 3,618
  • 1
  • 33
  • 41
  • "In most cases you know when you need it." Ha! That's what I thought! Now I discover that when I use DevExpress XtraGrid (a grid-like control) and bind it to a List<> of my objects, that internally the DevExpress code sometimes creates a Hashtable using my objects as keys. Then it crashes. Bah! – RenniePet Sep 11 '13 at 05:35