Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Overriding Object.Equals() instance method in C#; now Code Analysis/FxCop warning CA2218: "should also redefine GetHashCode". Should I suppress this?

I've got a complex class in my C# project on which I want to be able to do equality tests. It is not a trivial class; it contains a variety of scalar properties as well as references to other objects and collections (e.g. IDictionary). For what it's worth, my class is sealed.

To enable a performance optimization elsewhere in my system (an optimization that avoids a costly network round-trip), I need to be able to compare instances of these objects to each other for equality – other than the built-in reference equality – and so I'm overriding the Object.Equals() instance method. However, now that I've done that, Visual Studio 2008's Code Analysis a.k.a. FxCop, which I keep enabled by default, is raising the following warning:

warning : CA2218 : Microsoft.Usage : Since 'MySuperDuperClass' redefines Equals, it should also redefine GetHashCode.

I think I understand the rationale for this warning: If I am going to be using such objects as the key in a collection, the hash code is important. i.e. see this question. However, I am not going to be using these objects as the key in a collection. Ever.

Feeling justified to suppress the warning, I looked up code CA2218 in the MSDN documentation to get the full name of the warning so I could apply a SuppressMessage attribute to my class as follows:

    [SuppressMessage("Microsoft.Naming",
        "CA2218:OverrideGetHashCodeOnOverridingEquals",
        Justification="This class is not to be used as key in a hashtable.")]

However, while reading further, I noticed the following:

How to Fix Violations

To fix a violation of this rule, provide an implementation of GetHashCode. For a pair of objects of the same type, you must ensure that the implementation returns the same value if your implementation of Equals returns true for the pair.

When to Suppress Warnings

-----> Do not suppress a warning from this rule. [arrow & emphasis mine]

So, I'd like to know: Why shouldn't I suppress this warning as I was planning to? Doesn't my case warrant suppression? I don't want to code up an implementation of GetHashCode() for this object that will never get called, since my object will never be the key in a collection. If I wanted to be pedantic, instead of suppressing, would it be more reasonable for me to override GetHashCode() with an implementation that throws a NotImplementedException?


Update: I just looked this subject up again in Bill Wagner's good book Effective C#, and he states in "Item 10: Understand the Pitfalls of GetHashCode()":

If you're defining a type that won't ever be used as the key in a container, this won't matter. Types that represent window controls, web page controls, or database connections are unlikely to be used as keys in a collection. In those cases, do nothing. All reference types will have a hash code that is correct, even if it is very inefficient. [...] In most types that you create, the best approach is to avoid the existence of GetHashCode() entirely.

... that's where I originally got this idea that I need not be concerned about GetHashCode() always.

like image 795
Chris W. Rea Avatar asked Mar 25 '10 14:03

Chris W. Rea


People also ask

What is the reason for overriding Equals () method?

We can override the equals method in our class to check whether two objects have same data or not.

How do you override Equals method in object class?

Can we override the equals() method in Java? To compare two objects the object class provides a method with name equals(), this method accepts an object and compares it with the current object. If the references of these two objects are equal, then it returns true else this method returns false.

Should override Equals C#?

With value types, you should always override the == operator. Like the Equals method, the default implementation of the == operator uses reflection and is slow. Use the same logic as the Equals method, and you'll get much better performance when you're doing equality comparisons on value types.

What is an Equals method?

The Java String class equals() method compares the two given strings based on the content of the string. If any character is not matched, it returns false. If all characters are matched, it returns true. The String equals() method overrides the equals() method of the Object class.


1 Answers

If you are reallio-trulio absosmurfly positive that you'll never use the thing as a key to a hash table then your proposal is reasonable. Override GetHashCode; make it throw an exception.

Note that hash tables hide in unlikely places. Plenty of LINQ sequence operators use hash table implementations internally to speed things up. By rejecting the implementation of GetHashCode you are also rejecting being able to use your type in a variety of LINQ queries. I like to build algorithms that use memoization for speed increases; memoizers usually use hash tables. You are therefore also rejecting ability to memoize method calls that take your type as a parameter.

Alternatively, if you don't want to be that harsh: Override GetHashCode; make it always return zero. That meets the semantic requirements of GetHashCode; that two equal objects always have the same hash code. If it is ever used as a key in a dictionary performance is going to be terrible, but you can deal with that problem when it arises, which you claim it never will.

All that said: come on. You've probably spent more time typing up the question than it would take to correctly implement it. Just do it.

like image 88
Eric Lippert Avatar answered Oct 05 '22 23:10

Eric Lippert