UnitsNet: GetHashCode returns different hash for Equal units

Describe the bug .GetHashCode() returns different hash for 2 units for which .Equals() return true

To Reproduce

Length inCm = Length.FromCentimeters(100);
Length inM = Length.FromMeters(1);

Assert.AreEqual(inCm, inM);
Assert.AreNotEqual(inCm.GetHashCode(), inM.GetHashCode());

Expected behavior The GetHashCode() should return the same value because the .Equals() returns true

Additional context From the remarks:

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.

I’ve peeked in the sources and I think the issue is, that the value from which the unit is constructed is used when calculating the hashcode:

https://github.com/angularsen/UnitsNet/blob/7f67ffba914705eb27000b7d2962a90e0aebf70a/UnitsNet/GeneratedCode/Quantities/Length.g.cs#L1025-L1032

Shouldn’t this be normalized to the unit’s base unit or something along those lines?

Reason I’m asking is because I’m doing IEquatable implementations on my objects and in the .Equals(...) method, I’m comparing Units to a max of 5 decimals while in the GetHashCode() method I’m simply using the GetHashCode() method from the unit.

During the PR someone pointed out that for the GetHashCode(), the unit should also be rounded using the same logic as in the .Equals(...) method:

        /// <inheritdoc />
        public bool Equals(ResultAnalysis1DInternalForces other)
        {
            if (ReferenceEquals(null, other))
            {
                return false;
            }

            if (ReferenceEquals(this, other))
            {
                return true;
            }

            return base.Equals(other)
                && Equals(Member, other.Member)
                && Equals(MemberRib, other.MemberRib)
                && Index == other.Index
                && Section.UnitsNetEquals(other.Section)
                && N.UnitsNetEquals(other.N)
                && Vy.UnitsNetEquals(other.Vy)
                && Vz.UnitsNetEquals(other.Vz)
                && Mx.UnitsNetEquals(other.Mx)
                && My.UnitsNetEquals(other.My)
                && Mz.UnitsNetEquals(other.Mz);
        }

        /// <inheritdoc />
        public override bool Equals(object obj)
        {
            return ReferenceEquals(this, obj) || obj is ResultAnalysis1DInternalForces other && Equals(other);
        }

        /// <inheritdoc />
        public override int GetHashCode()
        {
            unchecked
            {
                int hashCode = base.GetHashCode();
                hashCode = (hashCode * 397) ^ (Member != null ? Member.GetHashCode() : 0);
                hashCode = (hashCode * 397) ^ (MemberRib != null ? MemberRib.GetHashCode() : 0);
                hashCode = (hashCode * 397) ^ Index;
                hashCode = (hashCode * 397) ^ Section.GetHashCode();
                hashCode = (hashCode * 397) ^ N.GetHashCode();
                hashCode = (hashCode * 397) ^ Vy.GetHashCode();
                hashCode = (hashCode * 397) ^ Vz.GetHashCode();
                hashCode = (hashCode * 397) ^ Mx.GetHashCode();
                hashCode = (hashCode * 397) ^ Mz.GetHashCode();
                return hashCode;
            }
        }
        public static bool UnitsNetEquals<TUnit>(this TUnit value, TUnit other)
        {
            if (value == null && other == null)
            {
                return true;
            }

            if (value != null && other != null && value is IQuantity thisUnit && other is IQuantity otherUnit)
            {
                try
                {
                    double thisValue = thisUnit.Value;
                    double otherValueInThisUnits = otherUnit.As(thisUnit.Unit);

                    return Comparison.Equals(thisValue, otherValueInThisUnits, ComparingConstants.DoubleComparisionDelta, ComparisonType.Absolute);
                }
                catch (ArgumentException e)
                {
                    return false;
                }
            }

            return EqualityComparer<TUnit>.Default.Equals(value, other);
        }

About this issue

  • Original URL
  • State: closed
  • Created 2 years ago
  • Comments: 16 (10 by maintainers)

Commits related to this issue

Most upvoted comments

I think the approach that @dschuermans is using in the ResultAnalysis1DInternalForces is the correct one- I’m also doing this for some of my classes- like when comparing two “measurements” (e.g. from an analytical balance)- I know what the resolution is, so I can safely implement a value based equality contract (think about rounding to the nearest nanogram). In other cases I known that the relative difference between the expected and the target concentration should not be more than some % - so I implement it accordingly (typically in a IEqualityComparer of the given class).

I don’t know if there is a reasonable default rounding precision that can be assumed when comparing using the default Equals/GetHashCode methods. By the way- I just recently discovered how Delphi 6 implements the SameValue function:

const
  FuzzFactor = 1000;
  ExtendedResolution = 1E-19 * FuzzFactor;
  DoubleResolution   = 1E-15 * FuzzFactor;
  SingleResolution   = 1E-7 * FuzzFactor;

function SameValue(const A, B: Double; Epsilon: Double): Boolean;
begin
  if Epsilon = 0 then
    Epsilon := Min(Abs(A), Abs(B)) * DoubleResolution;
  Result := Abs(A - B) <= Epsilon;
end;

However in #838 I didn’t actually go with the rounding approach- but was rather relying (at least initially) on the use of the IComparable interface - which is still not fixed (this test would fail in the current code-base).

This main problem with the IComparable approach was that it isn’t transitive - X.ToUnit(…).ToUnit(X.Unit) doesn’t necessarily convert back to X so- having a conversion function in the Equals method would seem like a bad idea.

As much as I’d like to have the proper value-based-unit-agnostic-equality-comparison-contract, it just doesn’t seem like it’s possible with the double arithmetic (I personally think that a decimal / rational type would have been better suited for use with real quantities- but that’s another can of worms).

As for fixing the Doubles- I think the best approach would be to have the strict equality contract (with an exception for Zero) and a Roslyn Analyzer that should generate a warning when ‘comparing floating point numbers’.

We can keep the interface, but I think the main argument is to conform to .NET conventions.

To avoid value boxing and support equality checks in generic collections is nice on paper, but I can’t really see a real world scenario for using strict value+unit equality checks myself. Except maybe trivial examples where the unit is always the same and values are not subject to rounding errors.

People should use the overload that allows to specify tolerance or passing in their own EqualityComparer, outside this interface.

But enough of that. Can we land on keeping IEquatable then, with strict equality checks. Alternative 1?

That sounds good to me!

Hi, will try to respond soon, need to digest it all first