Implementing Equals in C#

While working on refactorings, I often notice domain objects that implement Equals incorrectly.  Below is a sample implementation of Equals.  The key points are that Equals should not throw an exception if obj is null or of the wrong type.  Also note the different patterns for comparing reference and value members.  See pages 154-160 of Richter's excellent Applied Microsoft .NET Framework Programming for more details, including different patterns for reference or value objects:

public override bool Equals(object obj)
{
    if (obj == null) return false;

    if (this.GetType() != obj.GetType()) return false;

    // safe because of the GetType check
    Customer cust = (Customer) obj;     

    // use this pattern to compare reference members
    if (!Object.Equals(Name, cust.Name)) return false;

    // use this pattern to compare value members
    if (!Age.Equals(cust.Age)) return false;

    return true;
}

8 Comments

  • I particular like the following way of doing this:



    public override bool Equals(object obj) {

    Customer value = (obj as Customer);



    // assuming "Name" is the unique identifier for the object

    if (value == null)

    return false;



    return this.Name.Equals(value.Name)

    }



    I am not sure if maybe you added the "if (obj == null) return false;" statement before the cast if that would offer better performance ... something to check out ...

  • The as idiom only works for sealed classes. In your sample code, if obj is a ValuedCustomer, Equals will return true, which is wrong.

  • So what do you do about the ".. does not override Object.GetHashCode()" warning..?

  • Another problem with the 'value==null' test in Adam's post is that if the Customer class or one of its ancestors overrides the == operator, then the test 'value == null' could throw an exception. This depends on how the == method is written, but some implementations throw exceptions when presented with null argument(s). Thus the test 'value == null' is bad because the Equals(Object) method is not supposed to throw an exception. Replacing the test with '((Object)value)==null' would work, but if we are going to do that then we might as well test 'obj==null' at the top of the method. And if we're doing that, we might as well use the GetType() system in the original post. In summary, using the "as" idiom does not seem to be a good idea in methods that Equals(Object).

  • Instead of the null check followed by the type check you can write:

    if (!(obj is Customer))
    return false;

    And instead of casting you can use the "as" operator.


  • Hi guys, I think it's almost right though there are a couple of issues I see with the implementation.

    Use of the 'is' keyword is an implicit cast of which an 'as' explicit cast may as well be used as you're going to need it later on. Performing an is then an as is redundant. This wasn't part of the solution bu just explaining why it should be avoided.

    You should also be returning the "base" equals implementation if yours fails. In almost all cases it will fail, but as a practice you should rely on the base class to determine this, it could be that the base class has a bad implementation that could pass when it shouldn't, who knows, this could have been the desired design goal.

    I have a standard implementation that I have in my "Class" template in VS that looks like the following:

    ///
    /// Returns true if this object is equal to obj.
    ///
    /// Object you wish to compare to.

    /// true if this object is equal to obj.
    public override bool Equals(object obj) {
    //if (obj != null && obj.GetType().Equals(this.GetType())) {
    // Class1 other = obj as Class1;
    // if ((object)other != null) {
    // //TODO: Add Equals implementation
    // }
    //}
    return base.Equals(obj);
    }


    Simply uncommenting will provide you with a basis for property comparisons, eg

    public override bool Equals(object obj) {
    if (obj != null && obj.GetType().Equals(this.GetType())) {
    PocketIdentityKey other = obj as PocketIdentityKey;
    if ((object)other != null) {
    return other.Cabinet == Cabinet
    && other.Row == Row
    && other.Tier == Tier
    && other.SplitCode == SplitCode
    ;
    }
    }
    return base.Equals(obj);
    }

    Use of your GetHashCode implementation should always be consistant with your equals implementation as it affects the way a hashmap works, always perform an exclusive or of all properties that are being compared as a minimum, you may wish to perform a shift on some of the properties for a better uniqueness result.

  • another impl:

    if (objFoo.Foo.ToString().Equals(true))
    {
    chkFoo.Checked = true;
    }

  • I prefer checking typeof(T).IsInstanceOf(obj) rather than using GetType and an equality comparison.

Comments have been disabled for this content.