Testing Collections of Objects for Equality using IEquatable<T>

Question

I have a class that I have made implement IEquatable<T> so that when I'm testing I can easily compare IEnumerable collections of those objects using a call such as:

Assert.IsTrue(expected.SequenceEqual(actual));

This is currently working well but I have a few nagging doubts. The class looks like this:

public class ThirdPartyClaim : IEquatable<ThirdPartyClaim>
{
    // fields removed for question

    public bool Equals(ThirdPartyClaim compareTo)
    {
        if (object.ReferenceEquals(this, compareTo))
        {
           return true;
        }

        return this.ClaimId.Equals(compareTo.ClaimId) && 
               this.Firstname.Equals(compareTo.Firstname) &&
               this.Lastname.Equals(compareTo.Lastname);
    }

   public override int GetHashCode()
   {      
        int hashClaimId = this.ClaimId == null ? 0 : this.ClaimId.GetHashCode();
        int hashFirstname = this.Firstname == null ? 0 : this.Firstname.GetHashCode();
        int hashLastname = this.Lastname == null ? 0 : this.Lastname.GetHashCode();

        return hashClaimId ^ hashFirstname ^ hashLastname;
    }

My understanding of overriding the GetHashCode() is that it is used to compare objects pointing to the same instance of the class. It extremely unlikely that this will be required on this occasion (even in the future).

Is this understanding correct and if so, can I safely remove the code?

Is there a better way to compare collections of these objects in my unit test?

Although I'm constrained to use MSTest.

Thans

Answer 1

Overriding GetHashCode is required when you override Equals, otherwise hash-based containers might not work correctly. From the documentation of Object.Equals:

Types that override Equals must also override GetHashCode; otherwise, Hashtable might not work correctly.

Even though the code may not get exercised in your case, you should keep it anyway. In addition to being correct, it would help you test equality of collections regardless of their sequence:

Assert.IsTrue(expected.Except(actual).Count() == 0);    

One change I would make to the GetHashCode implementation is eliminating its symmetry: currently, switching around the first and the last name inside your object would lead to making the same hash code. This is sub-optimal. You can combine multiple ints int a hash code by multiplying them by a small prime number, e.g. 31, and adding them up, like this:

public override int GetHashCode()
{      
    int hashClaimId = this.ClaimId == null ? 0 : this.ClaimId.GetHashCode();
    int hashFirstname = this.Firstname == null ? 0 : this.Firstname.GetHashCode();
    int hashLastname = this.Lastname == null ? 0 : this.Lastname.GetHashCode();

    return 31*31*hashClaimId + 31*hashFirstname ^ hashLastname;
}