0

I extended the equals method and hashcode to check for equality of two identical objects with boolean properties. when I mutate the object making one of the boolean properties false instead of true it fails to recognize the difference and asserts they are equal;. Any Ideas why?

   public override bool Equals(object value)
    {
        if (!(value is LocationPropertyOptions options))
            return false;

        return Equals(options, value);
    }

    public bool Equals(LocationPropertyOptions options)
    {
        return options.GetHashCode() == GetHashCode();
    }

    public override int GetHashCode()
    {
        return ToString().GetHashCode();
    }

    public override string ToString()
    {
        return $"{Identifier}{AccountEntityKey}{Address}{Comments}{Contact}" +
               $"{Coordinate}{Description}{FaxNumber}{LastOrderDate}{PhoneNumber}" +
               $"{ServiceAreaOverride}{ServiceRadiusOverride}{StandardInstructions}" +
               $"{WorldTimeZone_TimeZone}{ZoneField}{CommentsOptions}";
    }
  • 5
    That's not a safe implementation of equality, ever. Two strings can be different and have the same hash code. You're also assuming that all of those values have sensible string implementations, and that they correlate entirely with equality, which likely isn't true, and either way, comparing the concatenated strings isn't a safe way of ensuring that all of the underlying values are the same. You need to compare the actual values. – Servy Oct 24 '18 at 14:05
  • 2
    Don´t use mutable members in an hashcode-implementation, at least not as ling as your instances are stored within a dictionary or a hashmap. Apart from this you shouldn´t rely on the hashcode for eauality. An equal hashcode doesn´t neccessarily mean two objects are equal, it´s just an *indicator*. You should dfinitly have a look at https://stackoverflow.com/questions/9317582/correct-way-to-override-equals-and-gethashcode – MakePeaceGreatAgain Oct 24 '18 at 14:07
  • Btw.: why do you re-direct a call to `bool Equals(object value)` to `Object.Equals(object, object)`, while `bool Equals(LocationPropertyOptions)` only uses the `GetHashCode`. Usually all your equality-comparisons should do the exact same thing. Everything else leads to high confusion. – MakePeaceGreatAgain Oct 24 '18 at 14:15

2 Answers2

1

You cast options from value, then call Equals with options vs value. That means you compare value with value, it returns always true for you

public override bool Equals(object value)
{
    if (!(value is LocationPropertyOptions options))
       return false;    
    return Equals(options, value);
}

Try comparing this with value, like

return Equals(this, value);
Antoine V
  • 6,998
  • 2
  • 11
  • 34
  • only thing is I have a robust unit test that compares the identical objects asserts true. Then mutates one object check again and asserts false equivalence which works. So this method is working – dylandotnet Oct 24 '18 at 15:51
1

This doesn´t really answer you question. However you should consider a few things when implementing equality to avoid this kind of error.

First you have two completely different implementations to indicate equality. Your override bool Equals(object value) redirects to the static method Object.Equals(object, object), which just performs a ReferenceEquals. Your public bool Equals(LocationPropertyOptions) (probably an implementation for IEquatable<LocationPropertyOptions>) on the other hand simply uses your strange GetHashCode-implementation, which is

Point two: you should not use a mutable member within your hashcode-implementation, in particular when your objects are stored within a dictionary or a hashmap, which heavily depends on a good implementaion for a hashcode. See MSDN on GetHashCode

You can override GetHashCode() for immutable reference types. In general, for mutable reference types, you should override GetHashCode() only if:

  • You can compute the hash code from fields that are not mutable; or

  • You can ensure that the hash code of a mutable object does not change while the object is contained in a collection that relies on its hash code.

Third and last: you shouldn´t use GetHashCode in your check for equality:

Do not test for equality of hash codes to determine whether two objects are equal

Whilst equal objects are assumed to have identical hashcodes, different objects may have the exact same hashcode anyway. An equal hashcode therefor is just an indicator that two instances may be equal.

Two objects that are equal return hash codes that are equal. However, the reverse is not true: equal hash codes do not imply object equality, because different (unequal) objects can have identical hash codes [...]
You should not assume that equal hash codes imply object equality.

MakePeaceGreatAgain
  • 35,491
  • 6
  • 60
  • 111