Search code examples
c#ienumerableequalsgethashcode

SequenceEqual in Equals makes GetHashCode broken


(1) I know that GetHashCode must return the same number for two objects if they are equal.

(2) I also know that SequenceEqual compare each value of a List, and Equals(list1, list2) will return true only if list1 and list2 are the same instance.

So, consider this code :

public List<ClassB> SampleList { get; set; }
public string Str { get; set; }
protected bool Equals(Uncorrectable other)
{
    return Enumerable.SequenceEqual(this.SampleList, other.SampleList) && string.Equals(this.Str, other.Str);
}

public override bool Equals(object obj)
{
    if (ReferenceEquals(null, obj)) { return false; }
    if (ReferenceEquals(this, obj)) { return true; }
    if (obj.GetType() != this.GetType()) { return false; }
    return this.Equals((ClassA) obj);
}

public override int GetHashCode()
{
    unchecked 
    { 
        return 
            ((this.SampleList != null ? this.SampleList.GetHashCode() : 0)*397) ^
            (this.Str != null ? this.Str.GetHashCode() : 0); 
    }
}

I really need this behavior((2) using SequenceEqual) for Equals, mostly for unit testing : make this code Assert.AreEqual(classA1, classA2) working.

But some of my code is probably broken because in this case

int hash1 = new List<ClassB>().GetHashCode(); 
int hash2 = new List<ClassB>().GetHashCode(); 

hash1 and hash2 are not equal.

So in my ClassA, (1) is not respected.

What is the best solution :

  1. Change ClassA.Equals method to use Equals(this.SampleList, other.SampleList) instead of Enumerable.SequenceEqual(this.SampleList, other.SampleList) and change all my tests
  2. Create another IEnumerable implementation where the Equals overriding is like SequenceEqual
  3. Modify ClassA.GetHashCode to call GetHashCode on all list items
  4. Do nothing
  5. Another one?

Solution

  • Simply don't base your GetHashCode on the SampleList: you don't need to use all the fields/properties in the GetHashCode().

    For example:

    unchecked 
    { 
        return 
            (this.Str != null ? this.Str.GetHashCode() : 0); 
    }
    

    or even better use only some information of the SampleList... The Count for example:

    unchecked 
    { 
        return 
            ((this.SampleList != null ? this.SampleList.Count.GetHashCode() : 0) * 397) ^
            (this.Str != null ? this.Str.GetHashCode() : 0); 
    }
    

    If you truly want, you can calculate the GetHashCode() on the elements of the SampleList.

    Now, for the C# obfuscation code tournament, 2016 edition:

    unchecked
    {
        return
            (397 * (this.SampleList != null ? 
                this.SampleList.Aggregate(0, (old, curr) => 
                    (old * 397) ^ (curr != null ? curr.GetHashCode() : 0)) : 
                0)
            ) ^
            (this.Str != null ? this.Str.GetHashCode() : 0);
    } 
    

    (please don't write it that way... Use a foreach cycle)