Inheriting from orekit detectors is impossible?

Orekit development
1

Hello,

Lately, we have encountered issues while inheriting from some orekit detectors because :

  • not all constructors are public
  • withHandler() method does not set the handler but add a new one instead

Constructors

Recently, we wanted to create a custom DateDetector, with some specific fields, on which a custom handler should be set. It is was not possible because the constructor with handler and dates is private, plus we do not have access to the list of dates to add them one by one.

withXxx() wording

Also, I have seen a lot of developers using the withHandler() method without reassigning the detector. It always takes a while to find that withXxx() methds return a new object.
I have always wondering if this wording withXxx() could be misleading?

For example, I have seen some developers using EclipseDetector calling detector.withPenumbra().withUmbra(). They believed that they were activating both penumbra/umbra events.

I think it would be easier if we could use a standard setter instead of withXxx() or let constructor at least protected. Maybe there is other ways ? Maybe there is something I did not understand on these features ?

Best regards,

Anne-Laure

2

Hi Anne-Laure,

It does seem that DateDetector should provide a method to get all the dates it is configured to detect. Perhaps List<AbsolutDate> getDates() {}.

Can you use composition instead of inheritance? E.g.

class MyDetector extends AbstractDetector<MyDetector> {
  DateDetector dateDetector;
  // ...
}

That would allow addition of custom fields in MyDetector and a handler and access to the DateDetector. That is the approach taken with BooleanDetector, EventShifter, and EventSlopeFilter.

If the use case doesn’t need to modify the g function then creating a custom handler would be simpler.

The withXxx() methods let the detectors be immutable. I’ve seen it called the “Fluent” pattern or “immutable setter” pattern. I can see how the .withPenumbra().withUmbra() sequence can be confusing. Perhaps renaming them so it reads .withPenumbraOnly().withUmbraOnly() would help? They are really setting a single value so another option could be e.g. .withBoundary(EclipseDetector.PENUMBRA).withBoundary(EclipseDetector.UMBRA) which perhaps makes it a bit clearer that it overrides the previous value?

I find the fluent pattern useful as it makes it easy to create copies. For example in the case of the eclipse detector once the umbra detector is set up it makes it very easy to get a copy that detects penumbra with all the same settings:

EclipseDetector umbra = ...; // long code to set up the detector how I like it
EclipseDetector penumbra = umbra.withPenumbra(); // easy to copy

Best Regards,
Evan

3

Hello Evan,

Yes composition would work if the dates getter were available.

About fluent pattern, I like it tremendously but usually I do not recreate the object. I have found on the first Google results that could be this way : Fluent interface - Wikipedia.

I think the word “with” is not explicit on the fact that it creates a new object.
Maybe it is a matter of habits :slight_smile:.
I only wanted to share this with you in case others users could have the same issues.

Thank you for this discussion!

Anne-Laure