What's the best strategy for Equals and GetHashCode?

Question

I'm working with a domain model and was thinking about the various ways that we have to implement these two methods in .NET. What is your preferred strategy?

This is my current implementation:

public override bool Equals(object obj)
{
    var newObj = obj as MyClass;

    if (null != newObj)
    {
        return this.GetHashCode() == newObj.GetHashCode();
    }
    else
    {
        return base.Equals(obj);
    }
}

// Since this is an entity I can use its Id
// When I don't have an Id, I usually make a composite key of the properties
public override int GetHashCode()
{
    return String.Format("MyClass{0}", this.Id.ToString()).GetHashCode();
}

Answer 1

Assuming that the instances are equal because the hash codes are equal is wrong.

I guess your implementation of GetHashCode is OK, but I usually use things similar to this:

public override int GetHashCode() {
    return object1.GetHashCode ^ intValue1 ^ (intValue2 << 16);
}

Answer 2

Domain-Driven Design makes the distinction between Entities and Value Objects. This is a good distinction to observe since it guides how you implement Equals.

Entities are equal if their IDs equal each other.

Value Objects are equal if all their (important) constituent elements are equal to each other.

In any case, the implementation of GetHashCode should base itself on the same values that are used to determine equality. In other words, for Entities, the hash code should be calculated directly from the ID, whereas for Value Objects it should be calculated from all the constituent values.

Answer 3

None of the answers here really hit the spot for me. Since you already said that you can't use Id for equality, and you need to use a bundle of properties, here's a better way to do that. Note: I do not consider this overall to be the best way to implement Equals and GetHashCode. This is a better version of the OP's code.

public override bool Equals(object obj) {
   var myClass = obj as MyClass;

   if (myClass != null) {
      // Order these by the most different first.
      // That is, whatever value is most selective, and the fewest
      // instances have the same value, put that first.
      return this.Id == myClass.Id
         && this.Name == myClass.Name
         && this.Quantity == myClass.Quantity
         && this.Color == myClass.Color;
   } else {
      // This may not make sense unless GetHashCode refers to `base` as well!
      return base.Equals(obj);
   }
}

public override int GetHashCode() {
   int hash = 19;
   unchecked { // allow "wrap around" in the int
      hash = hash * 31 + this.Id; // assuming integer
      hash = hash * 31 + this.Name.GetHashCode();
      hash = hash * 31 + this.Quantity; // again assuming integer
      hash = hash * 31 + this.Color.GetHashCode();
   }
   return hash;
}

See this answer by Jon Skeet for some of the reasoning behind this. Using xor is not good because various sets of data can end up resulting in the same hash. This wrap-around method with primes (the seed values of 19 and 31 above, or other values that you choose) does a better job of segmenting into "buckets" that have few collisions each.

If any of your values can be null, I encourage you to think carefully about how they should compare. You could use short circuit null evaluation and the null coalescing operator perhaps. But make sure that if nulls should compare as equal that you assign different hash codes to the different nullable properties when they are null.

Also, I'm not convinced that your Equals implementation makes any sense. When two objects are compared for equality, first their GetHashCode values are compared. Only if those are different is the Equals method run (so that if two objects that hash to the same value are different, this will be detected). Since your GetHashCode implementation doesn't refer to the base, it may make no sense for your Equals method to do so. Specifically, you will have a serious bug waiting to break things if Equals can return true for two objects whose hash codes are different.

Answer 4

I stumbled upon this old question and, IMHO, I didn't find any answer clear and simple stated the original question formulated by @tucaz.

I can agree with many considerations shared above (or below :D) but the «Question point» was missed (I think).

Provided that:

• Equality is required for entities
• Entity-Objects can be considered equals if they map the same entity, id est they refer to the same «Entity Key»
• The example shown by @tucaz just mention an «Id» (see the over-implemented GetHashCode())… not to mention the buggy Equals(…)

I can guess that one straightforward implementation could be:

public class MyEntity: IEquatable<MyEntity> {
    int Id;

    public MyEntity(int id){
        Id = id;
    }

    public override bool Equals(object obj) => Equals(obj as MyEntity);
    public bool Equals(MyEntity obj) => obj != null && Id == obj.Id;
    public override int GetHashCode() => Id;
}

That's all!

Answer 5

Hashcodes can collide so I don't think they are a good way to compare equality. You should compare the underlying values that make the objects "equal" instead. See @Jon Skeet's answer to this question: What is the best algorithm for an overridden System.Object.GetHashCode? for a better GetHashCode implementation if your equality encompasses several properties. If it's just a single property, you can just reuse it's hashcode.