search

unitsofmeasurement / indriya

JSR 385 - Reference Implementation
Other
122 stars 42 forks source link

NumberSystem must support Number types outside the JDK #229

Closed keilw closed 5 years ago

keilw commented 5 years ago

DefaultNumberSystem currently throws an IllegalArgumentException outside a small selection of Number subtypes, primarily those inside the JDK.

That is not a reasonable behavior, because there are other Number types defined by libraries like Apache Commons Numbers, GNU Math although that even defines Quantity and Unit in a rather weird way (Unit extends Quantity and both extend Numeric which is a Number), but other numeric types like RealNum often go beyond what we define here, so it should not be prevented to use another kind of Number.

andi-huber commented 5 years ago

Just to clarify: Any implementation of our NumberSystem interface can by design only support a closed set of concrete Number types. If users intend to use different or additional Number types, they also need to provide their own implementation of NumberSystem and configure Indryia to use it (by setting the static Calculus.NUMBER_SYSTEM field).

andi-huber commented 5 years ago

... this might be inconvenient for some use-cases, but still I'd rather have a reference implementation that does the arithmetic right (and fails early), than using a fallback which might involve precision loss, only to play nice with other libraries.

However, we could provide a second NumberSystem implementation, that does not throw these Exceptions, just to give users the option. But I would not recommend it, because I have a feeling it jeopardizes the purpose of this RI.

keilw commented 5 years ago

Can they do that without further extending e.g. NumberQuantity?

andi-huber commented 5 years ago

Yes, at least that's the intent.

keilw commented 5 years ago

It seems to do this only via a static member variable in Calculus, I probably would move this to the Indriya SPI similar to DimensionalModel Most people won't have to replace the DimensionalModel, but they could, and at the very least they should also be able to extend the NumberSystem.

andi-huber commented 5 years ago

Fine with me!