Hipparchus-Math / hipparchus

An efficient, general-purpose mathematics components library in the Java programming language
Apache License 2.0
142 stars 41 forks source link

Change RotationOrder from final class to enum. #72

Closed BryanCazabonne closed 4 years ago

BryanCazabonne commented 4 years ago

Hi all,

By looking at the RotationOrder class, I was a little bit surprise to see that this object is declare as final class instead of an enumerate.

I think it could be a good enhancement to set this class as an enumerate. Another improvement can be to add a static map (Map<String, RotationOrder> CODE_MAP) attribute in order to access a RotationOrder Object from a String value. Therefore, the following method can be also added:

public RotationOrder getRotationOrder(final String name) {
     final RotationOrder type = CODE_MAP.get(name);
     if (type == null) {
          // Exception
     }
     return type;
}

What do you think about this change? If you think this change interesting, I can perform the development.

I'm asking that because some attitude standard files (CCSDS ADM for instance) give the rotation sequence of Euler angles as a String value. Hence, thanks to my proposal, the RotationOrder object corresponding to the rotation sequence can be easily created.

Kind regards, Bryan

maisonobe commented 4 years ago

This seems fine to me, but I am not sure it would be binary compatible with earlier versions. It would be source compatible, though, as we can only use the public final constants XYZ, XZY... because the constructor is private.

The class is a a very old one. I traced it back from the Mantissa library in... september 2002. At that time, the current Java version was 1.4, but the library was still compatible with Java 1.3. Enumerates were not available before Java 1.5.

BryanCazabonne commented 4 years ago

I understand the problem of compatibility. Even if this change is minor in terms of development, it is important because it changes the nature of the class.

I performed the development and ask for a pull request (see #73). I let you decide if this change can be added or not.