Why should I not implement Equals and GetHashCode using reflection?

Question

I have some objects with a bunch of fields and I find myself having to implement GetHashCode and Equals. It is painful to go though each field manually so I wrote them like this:

public override int GetHashCode()
{
    int hash = 17;
    foreach (PropertyInfo p in GetType().GetProperties())
    {
        hash = hash * 23 + p.GetValue(this, null).GetHashCode();
    }
    return hash;
}

public override bool Equals(object obj)
{
    foreach (PropertyInfo p in GetType().GetProperties())
    {
        if (p.GetValue(obj, null) != p.GetValue(this, null))
            return false;
    }
    return true;
}

Other than speed considerations why shouldn't I implement them like this?

Answer 1

Here are a few reasons I would avoid this route

• It's much more reliable to compare fields instead of properties
• Your code makes the incorrect assumption that two objects are considered to be equal if they are the same reference (you are using ==). This is not the case as many types implement value equality via .Equals. It is very possible and legal for two different references to be considered Equals and would beat your test.
• If this form of Equality is used in a wide spread manner through your code base it will very easily lead to infinite recursion when the object graph has cycles.
• The GetHashCode method ignores that a property could be null

Below is a concrete example of a type which would cause infinite recursion in your application

class C1 {
  public object Prop1 { get; set; }
};

var local = new C1();
local.Prop1 = local;
var x = local.GetHashCode();  // Infinite recursion

Answer 2

Any value-type properties will be boxed by the GetValue calls, which means that they'll never compare as equal even if they have the same value.

You can avoid this by calling the static Equals(x,y) method -- which will then defer to the virtual x.Equals(y) method if necessary -- rather than using the non-virtual == operator, which will always test reference equality in this case.

if (!object.Equals(p.GetValue(obj, null), p.GetValue(this, null)))
    return false;