QuantityGenerator ignores ComparisonType in the obsolete Equals method

[gmkado]

We are in the process of migrating UnitsNet to the latest and found that some of our obsolete equals methods were failing. Inspecting the generated code shows that the comparison type is ignored.

https://github.com/angularsen/UnitsNet/blob/ce42aa89a7a0cac6f0f77ad28672f0769ac2376d/CodeGen/Generators/UnitsNetGen/QuantityGenerator.cs#L872

Seems like the comparison class still supports it, any reason to not keep it backwards compatible?

https://github.com/angularsen/UnitsNet/blob/ce42aa89a7a0cac6f0f77ad28672f0769ac2376d/UnitsNet/Comparison.cs#L60

[lipchev]

Given that the operations with doubles introduce rounding errors along the way, we decided (in v5) that it would be best if we deprecated the standard equality contract (A.Equals(B)) in favor of comparing with tolerance. Frankly, the problems with rounding errors run quite deeply- I wouldn't even trust the CompareTo operator (>=, <=): see #1367 for more details.

The good news is that I've been working on a solution to all these problems (no nuget available ATM). So I don't know how many warnings you've got after upgrading, but yeah- if you're not in a hurry, you might want to hold off on the "big-refactoring" until the official v6 release (there is currently a beta package, but that's no good either).

[gmkado]

Thanks for the quick response, I was reading through the various issues around comparisons and I understand the deprecation, but was there a reason to force ComparisonType.Absolute in the obsoleted method?

If you changed this line: https://github.com/angularsen/UnitsNet/blob/ce42aa89a7a0cac6f0f77ad28672f0769ac2376d/CodeGen/Generators/UnitsNetGen/QuantityGenerator.cs#L872

to

comparisonType: comparisonType);

Then it wouldn't change the behavior of existing (albeit problematic) comparisons, right?

[lipchev]

Oh I see what you mean, I think this must be a mistake 👍 :

        [Obsolete("Use Equals(Density other, Density tolerance) instead, to check equality across units and to specify the max tolerance for rounding errors due to floating-point arithmetic when converting between units.")]
        public bool Equals(Density other, double tolerance, ComparisonType comparisonType)
        {
            if (tolerance < 0)
                throw new ArgumentOutOfRangeException(nameof(tolerance), "Tolerance must be greater than or equal to 0.");

            return UnitsNet.Comparison.Equals(
                referenceValue: this.Value,
                otherValue: other.As(this.Unit),
                tolerance: tolerance,
                comparisonType: ComparisonType.Absolute);
        }

@angularsen we're not actually using the parameter (it's only used for in the xml-docs)..

[angularsen]

Looks like a mistake, anyone up for a pull request?

[lipchev]

Easy fix, now let's see if that "breaks" someone's code 😄

[angularsen]

Fix should be out shortly

Release UnitsNet/5.52.0 · angularsen/UnitsNet

[gmkado]

Wow, thanks for the quick work!