Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Was IQuantity.Value's return type intended to change to QuantityValue at v5? #1205

Closed
tmilnthorp opened this issue Feb 17, 2023 · 4 comments
Closed
Labels

Comments

@tmilnthorp
Copy link
Collaborator

At v5 IQuantity.Value now returns a QuantityValue type rather than double. Was this intended? It's a bit of an obscure change since IQuantity.Value is implemented explicitly, so only seen when you have a left-hand side type of IQuantity.

We have hundreds of places where we pass IQuantity.Value to an internal double method, and upgrading to v5 would require a cast to double on each one.

Changing it back to double does not break any tests FWIW.

@tmilnthorp tmilnthorp added the bug label Feb 17, 2023
@angularsen
Copy link
Owner

It was introduced here, to better support decimal quantities.
#1074

Previously, double was always returned, also for decimal quantities.

Implicit cast from QuantityValue to double was explicitly left out to force the consumer to accept the (possible) conversion from decimal to double, since you don't really know from IQuantity alone. We have IDecimalQuantity, we could have had IDoubleQuantity I guess, but the whole double/decimal design is already kind of smelly.

This may all be moot if we indeed remove decimal altogether, as discussed in #1194 (comment)

@tmilnthorp
Copy link
Collaborator Author

Fair enough. Code incoming :)

@angularsen
Copy link
Owner

waiting eating chips

@tmilnthorp
Copy link
Collaborator Author

I present #1207

angularsen added a commit that referenced this issue Feb 26, 2023
Ref #1205 

- Add `IValueQuantity<TValue>` extending `IQuantity` to expose
double/decimal types instead of `QuantityValue`
- Add `IQuantity<TUnitType, out TValueType>`, extending
`IValueQuantity<TValue>` with value specific `As(TUnitType)` method
- Mark `IDecimalQuantity` obsolete, replaced by above

---------

Co-authored-by: Andreas Gullberg Larsen <andreas.larsen84@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants