unitsofmeasurement / indriya

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

Implementing the Degrees (angle) unit #322

Closed imclem closed 3 years ago

imclem commented 3 years ago

Hi there,

First of all, thank you for this library, it's a real joy to use!

I'm trying to add a new type "Degree" to represent an angle in degrees.

If I define the unit like this:

final Unit<Angle> DEGREE_ANGLE = Units.RADIAN.multiply(180 / Math.PI);

assertEquals(57.2958, Quantities
        .getQuantity(1, Units.RADIAN)
        .to(Metrics.DEGREE_ANGLE)
        .getValue
        .doubleValue());

This does not produce the result that I expect (not even close), I get the following double value: 0.017453292519943295.

I also tried defined the unit like this:

final Unit<Angle> DEGREE_ANGLE = Units.RADIAN.multiply(180).divide(Math.PI);

But it produce the same result.

Now if I define the degree unit like this (thanks #257):

  final Unit<Angle> REVOLUTION  = Units.RADIAN.multiply(2.0 * Math.PI);
  final Unit<Angle> DEGREE_ANGLE = REVOLUTION.divide(360.0);

assertEquals(57.29577951308232, Quantities
        .getQuantity(1, Units.RADIAN)
        .to(Metrics.DEGREE_ANGLE)
        .getValue
        .doubleValue());

Everything works properly.

I tried debugging the converter method, but can't find why the first implementation is not working.

Why the first implementation is not working ? And is there a way to make it work ?

Thanks in advance

keilw commented 3 years ago

Comparing the double values is not always a good idea, but why don't you use DEGREE_ANGLE in the NonSI unit system implementation from https://github.com/unitsofmeasurement/si-units? There you will also find how it's done which is closer to the first approach but using precise enough values including the Rational number type instead of double or other primitives. You'll also find JUnit tests for it. I close this here, if you found any problems with the SI units or suggestions for improvements, please create a new ticket there.

imclem commented 3 years ago

@keilw I don't use it because I didn't know it existed... I will use it, thanks a lot !