Re: [Orekit Developers] Preparing release 8.0

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

Le 04/07/2016 à 11:48, Guilhem Bonnefille a écrit :
> 
> 
> Le 04/07/2016 11:25, Luc Maisonobe a écrit :
>> 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.
>>
> 
> It's clearly a matter of taste.
> 
> My understanding of the notion of "checked exception" is that the caller
> has to decided *explicitly* what to do with this exception.
> My understanding of the golden rule "catch an exception *only* if you
> have something to do with it" is only to avoid empty catch block.

An empty catch block is not equivalent to have nothing to do. It is
explicitly ignoring there was a problem by preventing the exception
to go upward. This works only if you are sure the error detected will
not break anything else later on, which is a rather strong assumption.

The equivalent to "have nothing to do with an exception" seem to me
to be catch it and rethrow it unconditionally, and I personnaly would
not like to see such code.

best regards,
Luc

> 
> But it's only my own understanding and feeling: I prefer explicit code.
> 
> At the same time, designing error management with exceptions is really
> really hard. The simplest example of this is the wide variety of
> different use cases in Java itself.
>