47
votes

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();
}
7
You can't use the results of GetHashCode as the only determinant in your Equals - hash codes can be the same when the objects are different. You'd be much better off comparing your Ids in Equals. For more on this, see Why is it important to override GetHashCode when Equals method is overriden in C#?Blair Conrad
You should keep in mind that GetHashCode() is mostly used in code where performance is important (lists with O(1) lookups, etc.). Your implementation is already rather slow, but you could speed it up already without changing much: return ("MyClass" + this.Id).GetHashCode(); (just something you might want to keep in mind with GetHashCode)Aidiakapi
@Aidiakapi Basing hashes on a concatenated string at all is probably a terrible idea.ErikE

7 Answers

37
votes

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.

12
votes

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.

6
votes

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);
}
4
votes

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!

3
votes

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.

1
votes

In addition to the answers (I am not allowed to write comments) I would like to point out that Visual Studio can autogenerate Equals and GetHashCode. See this answer: Is there a way to automatically generate equals and hashcode method in Visual Studio I was really looking for that custom implementation and didnt find it here.

I woulld also like to link this question: Best way to compare two complex objects It is about having a nested class structure. Down in the comments one can find the case for nested class structures with Enumerations (for example with List ).

0
votes

I want to look at some specific scenarios based on the answers above, and my own experiences.

A rule of thumb is, two instances with different hash codes should always be not equal, but if the have the same hash code they might or might not be equals. GetHashCode() is used to differentiate between instances quickly, and Equals() is used to verify equality (whatever that means to you).

Also a lot of built-in mechanisms look for an implementation of IEquatable<T> so it is a good idea to declare an override of Equals(MyClass) that actually does the checking.

Class with unique ID

Consider a class with a unique ID. Then the equals operation would just check the id. The same with the hash, which solely relies on the id.

public class IdClass : IEquatable<IdClass>
{
    public int ID { get; } // Assume unique
    public string Name { get; }


    #region IEquatable Members
    /// <summary>
    /// Equality overrides from <see cref="System.Object"/>
    /// </summary>
    /// <param name="obj">The object to compare this with</param>
    /// <returns>False if object is a different type, otherwise it calls <code>Equals(IdClass)</code></returns>
    public override bool Equals(object obj)
    {
        if (obj is IdClass other)
        {
            return Equals(other);
        }
        return false;
    }

    /// <summary>
    /// Checks for equality among <see cref="IdClass"/> classes
    /// </summary>
    /// <param name="other">The other <see cref="IdClass"/> to compare it to</param>
    /// <returns>True if equal</returns>
    public virtual bool Equals(IdClass other)
    {
        if (other == null) return false;
        return ID.Equals(other.ID);
    }

    /// <summary>
    /// Calculates the hash code for the <see cref="IdClass"/>
    /// </summary>
    /// <returns>The int hash value</returns>
    public override int GetHashCode() => ID.GetHashCode();

    #endregion

}

Class with properties

This case is similar to the above, but the comparisons depend on two or more properties, and the need to be combined asymmetrically in the hash code. This will become more evident in the next scenario, but the idea is if one property has hash A and the other property hash B, the result should difference from the case where first property has hash B and the other hash A.

public class RefClass : IEquatable<RefClass>
{
    public string Name { get; }
    public int Age { get; }


    #region IEquatable Members
    /// <summary>
    /// Equality overrides from <see cref="System.Object"/>
    /// </summary>
    /// <param name="obj">The object to compare this with</param>
    /// <returns>False if object is a different type, otherwise it calls <code>Equals(RefClass)</code></returns>
    public override bool Equals(object obj)
    {
        if (obj is RefClass other)
        {
            return Equals(other);
        }
        return false;
    }

    /// <summary>
    /// Checks for equality among <see cref="RefClass"/> classes
    /// </summary>
    /// <param name="other">The other <see cref="RefClass"/> to compare it to</param>
    /// <returns>True if equal</returns>
    public virtual bool Equals(RefClass other)
    {
        if (other == null) { return false; }
        return Name.Equals(other.Name)
            && Age.Equals(other.Age);
    }

    /// <summary>
    /// Calculates the hash code for the <see cref="RefClass"/>
    /// </summary>
    /// <returns>The int hash value</returns>
    public override int GetHashCode()
    {
        unchecked
        {
            int hc = -1817952719;
            hc = (-1521134295) * hc + Name.GetHashCode();
            hc = (-1521134295) * hc + Age.GetHashCode();
            return hc;
        }
    }

    #endregion

}

Value based class (structure)

This is almost identical to the case above, except being a value type (struct declaration) requires also re-definition of == and != to call equals.

public struct ValClass : IEquatable<ValClass>
{
    public int X { get; }
    public int Y { get; }

    #region IEquatable Members
    /// <summary>
    /// Equality overrides from <see cref="System.Object"/>
    /// </summary>
    /// <param name="obj">The object to compare this with</param>
    /// <returns>False if object is a different type, otherwise it calls <code>Equals(ValClass)</code></returns>
    public override bool Equals(object obj)
    {
        if (obj is ValClass other)
        {
            return Equals(other);
        }
        return false;
    }

    public static bool operator ==(ValClass target, ValClass other) { return target.Equals(other); }
    public static bool operator !=(ValClass target, ValClass other) { return !(target == other); }


    /// <summary>
    /// Checks for equality among <see cref="ValClass"/> classes
    /// </summary>
    /// <param name="other">The other <see cref="ValClass"/> to compare it to</param>
    /// <returns>True if equal</returns>
    public bool Equals(ValClass other)
    {
        return X == other.X && Y == other.Y;
    }

    /// <summary>
    /// Calculates the hash code for the <see cref="ValClass"/>
    /// </summary>
    /// <returns>The int hash value</returns>
    public override int GetHashCode()
    {
        unchecked
        {
            int hc = -1817952719;
            hc = (-1521134295) * hc + X.GetHashCode();
            hc = (-1521134295) * hc + Y.GetHashCode();
            return hc;
        }
    }

    #endregion

}

Note that struct should be immutable, and it is a good idea to add the readonly keyword in the declaration

public readonly struct ValClass : IEquatable<ValClass>
{
}