Clarify not mandatory values coming from CDM xml files

Orekit usage
1

Hello everyone!

I am trying to use Cdm object to read cdm xml files.

I see in the standard pdf that some parameters could be not mandatory.

I see Orekit translate this absence by not initializing the parameters values.
Most of these parameters (all?) are java primitive class field, for example in ODParameters:

    /** The number of observations accepted for the OD of the object. */
    private int obsUsed;

How can you distinguish the 0 value from absence of parameter ?
This is even more dangerous in the case of boolean parameters (the default java primitive value is false).

We often use Optional in that sort of cases in our code. Do you think it would make Orekit more explicit on these not-mandatory parameters ? If not, do you have any other solution so we could distinguish if the parameter is defined and if so, be confident with the returned value.

Best regards,

Anne-Laure

2

Hello @Anne-Laure,

Optional is very nice but I think it may turn the code and its usage a bit heavy.
I may be wrong though and I have seldom used the Optional class.
What would you think of using the class types instead of the primitive types (e.g. Boolean instead of boolean), setting them to null (or NaN for Double) when the optional value isn’t there?

Best regards,
Maxime

3

Hello,

Thank you for the reply!

I strongly fear Nan : they spread through computations… until giving “strange” results at the end, with laborious work to find where Nan value has been introduced at first.

And I am not a fan of null objects in general.

Why Optional is better than null objects ?

  • Code is explicit
    I am used to distinguish in my code parameters that are expected to be defined (fastly sending null pointer exception if not) and the others.

  • Code is not verbose
    Optional makes explicit the absence of information and easier to handle than null :

cdm.getDataObject1().getODParametersBlock().getObsUsed().ifPresent(value -> doSomething(value))

   int obsUsed = cdm.getDataObject1().getODParametersBlock().getObsUsed();
   if(obsUsed != INT_ABSENT_VALUE){
       doSomething(obsUsed )
   }

Using or not Optional might be a matter of habits and I could understand that Orekit keeps conservative on this point. Null solution would at least fit the CSSDS definition.

Migrating CCSDS with Optional would be a big work, I am not sure that I could find some “time” but if so, I would be glad to contribute!

Best regards,

Anne-Laure

4

I understand and you have good arguments @Anne-Laure.
It’s true that it looks much less cumbersome and verbose than I initially thought.
Optional seems to be the best way to handle the optional values in CCSDS files (just writing it makes it obvious :D)
It would be a huge work though, as you mentioned.
Maybe some other users/developers could share their thoughts on this.
Maybe @luc, who developed and reworked most of the CCSDS package?
As for me, I’m ok with it.

Best regards,
Maxime

5

My answer is not influenced by the fact that Anne-Laure is located 10 meters in front of my office :slight_smile:

Usually, I’m not in favor of using Optional in Orekit because I find that they can increase the complexity of a low level library in terms of development and use. But for CCSDS I think they can be very useful to handle the mandatory (or not) keys.

The main drawback here is that it represents a lot of work…

Bryan

6

I am not a big fan of Optional.
For double types, I prefer NaN (and I do this precisely because they spread all around), and -1 for integers that are intended to be counts.

7

Hello,

@luc I understand your point. I don’t think Optional should be used extensively in Orekit, especially in the classes modeling the physics of the FD.
However, for parsing files, I think they have their place. Especially in CCSDS where many fields are optional. In my opinion, this could lead to a more elegant design. There wouldn’t be any need to document the optionality of a parameter since it would already be embedded in the signature of the variable.
And if the mandatory field of a parameter is changed with a new CCSDS version, Optional will help us make sure that this change is propagated everywhere in the code.
Regarding your examples, I think integers can be negative in CCSDS entries so the “-1-method” won’t work.
The other option I proposed would be to use class types Integer, Double etc. but as Anne-Laure showed in her example this will lead to adding a lot of (if parameter != null) everywhere in the code.

1 Like
8

OK.
You convinced me.

+1 for Optional

9

I fully agree with Maxime’s remark. And also think that CSSDS is the only place in Orekit where Optional can improve the understanding and use of the code.

@Anne-Laure could you open an issue in the Gitlab repository?

10

Thank you every one!
The issue is created here : https://gitlab.orekit.org/orekit/orekit/-/issues/1103

Best regards,

Anne-Laure