IQuantity with QuantityValue as Value type (PoC) #1124
Conversation
- QuantityValue implements most of the INumber interface members / operators (operations preserving decimal when possible, failing back to double, clamped to the real values- infinities excluded) - changed the type of IQuantity.Value/As to QuantityValue (explicit cast required) - added back the IEquatable interface: behavior depending on the runtime type of the quantity values (strict equality comparison for double/double) - added back the fix for the non-reflexive CompareTo issue (and the corresponding tests) - added a check preventing the possible stack overflow exception when calling ToUnit() with default(TUnit) (or any other invalid Enum value) - DataContracts changed to reflect the possible use of either double or decimal as ValueType (the Json converters were not changed- but probably should be) - removed the obsolete Json converter tests
|
I haven't looked at it very closely yet, but isn't this now conflicting with #1074 ? |
…rialization.JsonNet.CompatibilityTests.csproj from the build script
|
@pgrawehr Yes, quite so I'm afraid- I actually started off with the implementation from your branch, but things evolved quickly from there - up to the point where It's now a case of either or neither. |
|
Ah, damn it- I just re-read what I wrote last night, realizing that the equality contract is still broken- due to the lack of transitivity (again).. If we wanted to be strict in that regard we'd have to return |
Codecov Report
@@ Coverage Diff @@
## release/v5 #1124 +/- ##
=============================================
- Coverage 85% 39% -46%
=============================================
Files 315 310 -5
Lines 48751 46593 -2158
=============================================
- Hits 41618 18445 -23173
- Misses 7133 28148 +21015
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Then, to make it easier to compare and to comply with the license, please commit your changes separately and do not squash contributions from different users to one commit. |
I lost a bit momentum on this one after getting backlash on the change, but I personally think strict equality is the better compromise after ages of discussions. If you agree, please help me review the PR so we can get it merged. ✨ Add back IEquatable with strict equality PR 1100 Original discussion and decision for strict equality: |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
As per our discussion over at #875 here's the proof of concept, based on the explicit struct layout proposed by @AndreasLeeb and @pgrawehr (could you please check my modification - I had to make the fields nullable in order to avoid having the DataContractSerializers spew the trash value produced by the overlapping bits).
QuantityValueimplements most of theINumberinterface members / operators (operations preserving decimal when possible, failing back to double, clamped to the real values- infinities excluded) (see the QuantityValueTests and the QuantityComparisonTests)IQuantity.Value/AstoQuantityValue(explicit cast required)IEquatableinterface: behavior depending on the runtime type of the quantity values (strict equality comparison for double/double)CompareToissue (and the corresponding tests)ToUnit()withdefault(TUnit)(or any other invalid Enum value)Obviously this is quite a breaking change:
Mass.FromKilograms(1)- the implicit conversion is to decimal instead of double, but I think is the correct behavior: here's an example where the resulting type is decimal, failing back to double as we get into the astronomical units (in contrast to this other example - where all values are doubles). I've intentionally left out a margin for the range check, as to avoid having such units loose all of their precision.ToDouble/ToDecimalmethod) is required- but there is no way around that.IDecimalQuantityinterface (which currently stands for "a quantity using decimal-based conversion functions"- not sure what we want to do about that..).IEquatableimplementation seems to be correct although possibly confusing-Mass.FromKilograms(1m) == Mass.FromGrams(1000d),Mass.FromGrams(1000d) == Mass.FromGrams(1000d).Mass.FromKilograms(1d) != Mass.FromGrams(1000d).