Re: [Orekit Developers] Preparing release 8.0

[Luc Maisonobe <Luc.Maisonobe@c-s.fr>]

Le 04/07/2016 à 11:13, Guilhem Bonnefille a écrit :
> 
> 
> Le 04/07/2016 10:49, Luc Maisonobe a écrit :
>> Hi Guilhem,
>>
>> Le 04/07/2016 à 09:44, Guilhem Bonnefille a écrit :
>>>
>>> Le 24/06/2016 17:10, MAISONOBE Luc a écrit :
>>>> This last change would be to get rid of the PropagationException and
>>>> replace it with a simple OrekitException. The rationale for this change
>>>> is that PropagationException failed to bring any value and just added
>>>> complexity. A typical case is used code to implement a step handler.
>>>> Such code can only throw PropagationException, not the superclass
>>>> OrekitException. However, since that code often uses other Orekit
>>>> features (to get PVCoordinates in some frame for example), then users
>>>> must wrap the OrekitException thrown from below. We end up with lots
>>>> of code like:
>>>>
>>>>   try {
>>>>     // do some Orekit stuff
>>>>   } catch (PropagationException pe) {
>>>>     // already the good sub-type of exception, just re-throw
>>>>     throw pe;
>>>>   } catch (OrekitException oe) {
>>>>     // wrap the underlying exception
>>>>     throw new PropagationException(oe);
>>>>   }
>>>>
>>>> Do you think we could remove this PropagationException? 
>>> I'm not sure to understand the change: will the step handler declared
>>> throwing an OrekitException or nothing?
>> Yes, the step handler is declared to throw an OrekitException. It is
>> indeed a checked exception, so decraling it is mandatory.
>>
>>> If the PropagationException is replaced by OrekitException, doing such
>>> change will bring the developer face to a situation where exception can
>>> break the logic of the code without being warned because OrekitException
>>> are naturally allowed by the signature of step handler.
>> I don't understand what you mean here. In any case, the exception was
>> allowed and declared, the change is only that we use a single class
>> instead of two.
>>
> 
> Here is my understanding.
> 
> When a developer write a new step handler, he has to implement a method
> already declared to throw OrekitException. When writing the code of this
> method, if a call throws an OrekitException, the compiler/IDE will not
> alert the developer because a "solution" exists: the exception is
> directly thrown, silently.
> 
> Of course it is "simpler" for the developer beause he has nothing to
> write to handle this. But in the same time, he is not informed of the
> solution selected by the compiler.
> 
> I feel this is not the expected behavior of a checked exception. Seems
> paradoxal.

I don't agree. For me, it is the expected behaviour. You catch an
exception *only* if you have something to do with it. It at the current
code level you cannot manage it, then you must let it go through.

Monitoring exception at each level would look to me just as the old
fortran method with error codes and source program crippled with

  call function()
  if (ier .ne. 0) then
    return
  endif

  call otherFunction()
  if (ier .ne. 0) then
     return
  endif

With exceptions, errors are managed only at two levels: where they
are created (an if statement to detect the error condiion and
a throw) and where they are managed (a catch statement and a
corresponding action).

Many programs cannot manage exception at intermediate level so
error catch and display is managed only at the top level. It is
just one place, near the main function.

Note that the previous behaviour was exactly the same. If lower
level code did throw a Propagationexception directly, it could
have gone upward without the step handler noticing it because the
compiler would have allowed a code that does not attempt to
catch it in between.

> Look: if every method in Orekit do the same, there will be no difference
> of behavior between an OrekitException and an unchecked exception. ;-)

No. The difference is that the developer is aware of a checked
exception. If he/she writes:

  public void handleStep(...) {
  }

It will work well only if no exception is thrown in the body. If an
OrekitException is thrown, user will need to declare it. With the
change, it is allowed to declare it. with the previous version, user
had to convert the exception type, which was a pain.

best regards,
Luc

>