Overriding Object.Equals() instance method in C#; now Code Analysis / FxCop warning CA2218: "should
- by Chris W. Rea
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.
