✨ Add back IEquatable with strict equality

[angularsen]

Fixes #1017

• Reverted removing IEquatable.
• Changed the implementation to strict equality on both Value and Unit.
• Improved tests.
• Improved xmldocs for equality members.
• GetHashCode() left unchanged, which includes quantity name in addition to value and unit, so that LengthUnit.Meter = 1 and MassUnit.Gram = 1 are still considered different in collections of IQuantity.

Reverts commit f3c7e25.
"🔥 Remove IEquatable and equality operators/methods"

[angularsen]

@dschuermans I implemented strict equality, if you want to look over it.

Based on decision in #1017 (comment).

[dschuermans]

Is it just me, or is this confusing wording?

Shouldn't it be:

Returns true if Value and Unit are exactly equal for both quantities?

[angularsen]

This is for != not equals, but yes I agree it was clumsy wording.

I propose these:
==

Returns true if both <see cref=""Value"" /> and <see cref=""Unit"" /> are exactly equal for both quantities.

!=

Returns true if either <see cref=""Value"" /> or <see cref=""Unit"" /> are not exactly equal for both quantities.

[dschuermans]

@dschuermans I implemented strict equality, if you want to look over it.

Based on decision in #1017 (comment).

Nice,

But if I understand correctly, the case I made in the mentioned issue still won't work right? (1m equals 100cm and thus should yield the same hashcode)

[angularsen]

@dschuermans I implemented strict equality, if you want to look over it.
Based on decision in #1017 (comment).

Nice,

But if I understand correctly, the case I made in the mentioned issue still won't work right? (1m equals 100cm and thus should yield the same hashcode)

No, 1m and 100cm are no longer equal as of this PR. Their units differ. So GetHashCode does not have to be equal for these either.

Or did I misunderstand?

[dschuermans]

@dschuermans I implemented strict equality, if you want to look over it.
Based on decision in #1017 (comment).

Nice,
But if I understand correctly, the case I made in the mentioned issue still won't work right? (1m equals 100cm and thus should yield the same hashcode)

No, 1m and 100cm are no longer equal as of this PR. Their units differ. So GetHashCode does not have to be equal for these either.

Or did I misunderstand?

Erm, for me personally that sounds wrong if UnitsNet will now report that 1m is not the same as 100cm.

Imagine that you're filling out forms to get a building permit and you provide your plan which has values in cm whereas the city's system works in m and your request for a permit gets denied because the clerk tells you "well, we only allow this to be a max of 4m wide and your application states 400cm"

But in any case, if i recall my initial question was that if it was possible to globally configure the default comparison's number of decimals, so that I wouldn't have to use the overload everywhere in code.

We "fixed" this by using an extension method which allows us to configure the number of decimals in a single spot
I then bumped into the issue with the hash codes not being the same (1m has different hash then 100cm) but in our case, those values are the same.

So for me and my project, things could've stayed the way they were, as I created work-arounds for it.
We're still on 4.72 and as long as everything works like it's supposed to we're good.
I just hope we don't need any new units, as I do not wish to upgrade to something that'll say that 1m.Equals(100cm) is false 😅

[codecov]

Codecov Report

Base: 86% // Head: 87% // Increases project coverage by +0% 🎉

Coverage data is based on head (be24b22) compared to base (ed3da18).
Patch coverage: 99% of modified lines in pull request are covered.

Additional details and impacted files

@@           Coverage Diff            @@
##           master   #1100     +/-   ##
========================================
  Coverage      86%     87%             
========================================
  Files         321     321             
  Lines       48165   49803   +1638     
========================================
+ Hits        41699   43337   +1638     
  Misses       6466    6466             

Impacted Files	Coverage Δ
UnitsNet/GeneratedCode/Quantities/Speed.g.cs	92% <ø> (+<1%)	⬆️
...t/GeneratedCode/Quantities/StandardVolumeFlow.g.cs	85% <ø> (+<1%)	⬆️
UnitsNet/GeneratedCode/Quantities/Temperature.g.cs	85% <ø> (+<1%)	⬆️
...eneratedCode/Quantities/TemperatureChangeRate.g.cs	85% <ø> (+<1%)	⬆️
...Net/GeneratedCode/Quantities/TemperatureDelta.g.cs	85% <ø> (+<1%)	⬆️
.../GeneratedCode/Quantities/TemperatureGradient.g.cs	83% <ø> (+<1%)	⬆️
.../GeneratedCode/Quantities/ThermalConductivity.g.cs	79% <ø> (+1%)	⬆️
...et/GeneratedCode/Quantities/ThermalResistance.g.cs	84% <ø> (+<1%)	⬆️
UnitsNet/GeneratedCode/Quantities/Torque.g.cs	90% <ø> (+<1%)	⬆️
...sNet/GeneratedCode/Quantities/TorquePerLength.g.cs	89% <ø> (+<1%)	⬆️
... and 126 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[angularsen]

I hear your pain @dschuermans 😅 This discussion has literally gone for years, on and off.

I think the reasoning is best outlined in this comment #1017 (comment)

I simply don't see a trivial solution here that makes everyone happy.
Strict equality seems to be the lesser evil. People will run into subtle bugs with option 2, trying to apply some rounding on the equality for various sizes of values and units - plus GetHashCode() seems tricky to get right with rounding.
Strict equality is at least explainable, but I agree not intuitive.

As summarized in the comment:

If the value + unit doesn't match exactly, they are not equal.
Simple, arguably correct, will 100% piss off consumers trying to equate 100 cm to 1 meter.

Static config of equality

I guess we could allow for a static configuration of a default IEqualityComparer, where you can configure the equivalent of calling one of these:

Length.FromMeters(1).Equals(Length.FromCentimeters(100), 1e-5, ComparisonType.Absolute) // 1m +/- 0.00001
Length.FromMeters(1).Equals(Length.FromCentimeters(100), 1e-3, ComparisonType.Relative) // 1m +/- 0.1% 

At least then you are making an informed choice on the rounding.
However, GetHashCode() would still produce values for strict equality in that case, and differ as you described in your issue.

If you see a better solution or have convincing arguments for option 2 or 3, then I'm all ears.

[dschuermans]

Well, one solution I see here (not sure if it's technically feasible) is to have 2 separate packages of UnitsNet:

1. Uses strict equality and produces the same hash code for units that are 100% equal. The consumer of the package should be informed that comparing Length.FromMeters(1) does not equal Length.FromCentimeters(100)

2. Uses sane equality and produces the same hash code for units that are equal in a sane way. The consumer of the package is informed of rounding errors that may occur when comparing 2 units which differ a lot in quantity (eg terameter vs nanometer)

On the other hand,
The static config for equality sounds as a nice addition, maybe that something that can be done for GetHashCode too?
Maybe not even static, but it could be provided when constructing the unit?
e.g

Length unit1 = Length.FromMeters(1, Equality.Sane);
Length unit2 = Length.FromCentimeters(100, Equality.Sane);
unit1.Equals(Unit2); // returns true

Length unit3 = Length.FromMeters(1, Equality.Strict);
Length unit4 = Length.FromCentimeters(100, Equality.Strict);
unit3.Equals(Unit4); // Returns false;

Store the equality setting on the unit itself, it could then be used inside the Equals and GetHashCode methods accordingly.
I can already hear you ask "What if you try to compare a sane with a strict unit?"
In that case, I'd throw an exception and simply not allow it.
You are in control of how you use UnitsNet in your project, so you get to choose if you use strict or sane 🤷‍♂️

public bool Equals({_quantity.Name} other)
{{
    if(equalitySetting != other.equalitySetting){
       throw new NotSupportedException("You cannot compare strict units with sane units");
    }
    switch(equalitySetting) 
    {
        case Equality.Strict:
            return new {{ Value, Unit }}.Equals(new {{ other.Value, other.Unit }});
        case Equality.Sane:
        {
            double thisValue = this.Value;
            double otherValueInThisUnits = other.As(this.Unit);

            return Comparison.Equals(thisValue, otherValueInThisUnits, 1e-05, ComparisonType.Absolute);
        }
    }
}}

public override int GetHashCode()
{
    switch(equalitySetting)
    {
        case Equality.Strict:
        {
            return new { Info.Name, Value, Unit }.GetHashCode(); 
        }
       case Equality.Sane:
       {
            var valueOfUnitAsBaseUnitRounded = Math.Round(this.As(this.QuantityInfo.BaseUnitInfo.Value), 5, MidpointRounding.AwayFromZero);

		return (new
		{
			this.QuantityInfo.Name,
			valueOfUnitAsBaseUnitRounded,
			this.QuantityInfo.BaseUnitInfo.Value
		}).GetHashCode();
       }
    }
}

[angularsen]

Thank you for the comments and suggestions.

2 separate packages of UnitsNet

I don't like this option. It will confuse people what package to choose, and it complicates transient dependencies on UnitsNet, where libraries A and B take in two different UnitsNet nugets. It also feels overkill to solve a rather small problem.

Uses sane equality and produces the same hash code for units that are equal in a sane way.

This is the challenge. How do we produce the same hash code for units that are approximately equal, for very small or very large values and when comparing very small units to very large units?

Maybe not even static, but it could be provided when constructing the unit?

Not sure I favor this approach. Consumers may run into runtime exceptions if quantities happen to be constructed with different equality settings, such as getting quantities computed by a library out of your control.

I am not a fan of static config either, but if we had to choose between ctor parameter and static config, I would in this case prefer the latter. I'm still not convinced we should offer the static config though, since we would still have to solve the challenge about the hashcode mentioned above.

[angularsen]

Some other options not mentioned:

• For comparison methods on collections, you can pass IEqualityComparer to provide your own equality comparers. UnitsNet could maybe provide some default instances based on typical relative and absolute comparison.
• The consumer can write an extension method to make it syntactically easy to compare two quantities with their own equality comparer. UnitsNet could possibly also provide overloads or extension methods for typical relative/absolute comparison.

However, none of these really address the problem that many will expect 100 cm to equal 1m out of the box.

[angularsen]

The current discussion seems to have met a dead end, I propose to move ahead with the strict equality change and rather discuss any ideas for global configuration in a separate issue.

[lipchev]

I've suggested fixes for the comments on the Equals overload.
As for the question of the precision on the custom tests- I have no suggestions :)

[angularsen]

Sorry for the late reply, I have been missing out on email notifications for some time and also been very busy.

Will review soon.