Re: [Orekit Developers] Data-related exceptions

"LUGAN, Anne-Laure [FR]" <anne-laure.lugan@airbus.com> a écrit :

Hello every one, I have been using unchecked exception more and more in the late years... and I have no regret about it! Against my position, I would like to quote that Eclipse can help to remove partially OrekitException. You can tune Eclipse to raise a compilation warning if the exception is not thrown in the method. Still, it means, if we migrate only a few OrekitException to unchecked, that we have to remove the "throws OrekitException" one by one, ie each time Eclipse raises a warning through all hierarchy call. I agree that it would be more helpful to have a few more meaningful Exception than the single OrekitException.

A handful of exceptions is probably useful, but we should not go to the extrem of having one exception for each error. This is what was selected for Apache Commons Math (after months of harsh discussions that I would really not like to have again). It was almost the first thing we removed when creating Hipparchus. My point at that time is that a library should have a top level exception (or two, one for checked and one for unchecked exceptions) so people developing top level applications and not aware of a rich exceptions hierarchy can still do a "catch (TheTopLevelException e)" if they want, and they will catch everything. As some of our algorithms depend on a large number of lower level algorithms, using too many dedicated exceptions would lead people using for example orbit determination to need to catch errors linked to propagation, time, force models, JPL ephemerides, EOP, measurements, frames, data loading... Using the standard Java hierarchy (IOException, ArrayIndexOutOfBoundException, ParseException, ...) would prevent such a catch all strategy (there's no multiple inheritance in Java), so I am not comfortable with this choice. As a summary, I think we have seen the following proposals in this thread: 1) change a few checked exceptions to unchecked 2) change all checked exceptions to unchecked 3) use standard java exceptions (IOException, ...) 4) create a few different Orekit exception for different errors 5) use a small Orekit hierarchy with an easy to catch top level Recalling previous messages I would say the various positions are: Yannick : 1 + ? Luc : hesitating between 1 + 4 + 5 or 2 + 4 + 5 Evan : 2 + ? Guilhem : ? + 3 Anne-Laure: 2 + 4

I am leaning towards just 2, though I am not opposed 1. I am a little wary of the other options, but would not oppose them if the community wants to move in that direction. I would like to avoid ending up with a large number of Orekit specific exceptions as commons math did. I recognize that my usage of Orekit in a research oriented setting is not representative of the whole Orekit community, but the following are my experience using the exceptions.

Any time I need to know which error occurred (opposed to just that a general Orekit error occured) I catch OrekitException and compare getSpecifier() to the enum in OrkeitMessages. This allows the developer to know what went wrong at a low level while only having one top level exception. I can remember using this pattern in the past, but after a quick grep through my projects I don't see this pattern in use any more.

Over 90% of the places where I catch an OrekitException in my code I simply wrap it in a RuntimeException and re-throw it. This is needed to work with other Java libraries that do not depend on Orekit, e.g. Hipparchus and Java 8 streams. Most of the time I don't mind, but it does make writing lambdas much more verbose. In FunctionalDetector I had to create GFunction, a specialized version of ToDoubleFunction that throws an Orekit exception to make it easier for a user to write a lambda.

The only location in my code that I could find where I handled an OrekitException without wrapping it and immediately re-throwing it was at the beginning of a program to check if leap seconds and EOP were loaded. In that case it would catch the exception, print out detailed instructions on how to set the Orekit data path correctly and then quit.

All this to say that I don't think Orekit needs more than one checked exception type, one runtime exception type, and one error exception type. And from my perspective one runtime type and one error type would be sufficient.

One other concern that I have is that I don't want to create a large amount of work for the Orekit maintainers. To me the currently implementation is good enough, if a little rough around the edges but it doesn't warrant a large effort to rewrite significant amounts of code.

best regards, Luc

Best regards, Anne-Laure -----Original Message----- From: orekit-developers-request@orekit.org [mailto:orekit-developers-request@orekit.org] On Behalf Of Guilhem Bonnefille Sent: Monday, July 02, 2018 5:44 PM To: orekit-developers@orekit.org Subject: Re: [Orekit Developers] Data-related exceptions Le 02/07/2018 à 14:35, MAISONOBE Luc a écrit :
"Ward, Evan" <Evan.Ward@nrl.navy.mil> a écrit :
Hi Yannick, On Mon, 2018-07-02 at 08:56 +0000, JEANDROZ, Yannick [FR] wrote: I believe this change would allow for a more lightweight code for Orekit users. What are your thoughts about this proposition ? If there is a consensus, I could push the analysis further and begin to tinker with Orekit code. I am OK with making OrekitException a RuntimeException.
Wow, this would be a drastic change! For sure, it would solve Yannick (and many others) problem. What I like in checked exception is that you don't get surprised, the compiler prevents you from creating code that does not check for errors. However, I do agree that this can go out of hands if the code has many internal checks and errors can be raised almost everywhere. So I admit that we failed at some points and that as Yannick writes, almost every method throws an OrekitException. Therefore, the current code is crippled with throws declaration in too many places. So... I'm on the fence on being convinced to follow Evan drastic suggestion. What do other people think about this? Could someone push a little harder to convince old school developers like me? If we go this way, should we directly remove the "throws OrekitException" declarations and remove the corresponding Javadoc? I think we should, because with unchecked exceptions, the tools (IDE, compiler, checkstyle...) will not help us maintain the consistency of such declarations, and it will soon become inconsistent with underlying code.
I share the Luc's feeling: checked Exceptions are not bad by design and cannot be replaced by unchecked ones. In order to gain full benefit of exceptions, they require lot of attention, a precise comprehension of use cases and a neat design. I feel that the current issue is that Orekit should use a more refined tree of exceptions or use native/standard exceptions, in order to distinguish different situations and allow each conceptual level to decide what to do with witch situation. But this needs lot of work (575 occurrences of "throw new OrekitException"). For example, many classes related to file parsing should throw native IOException (or inherited ones). When dealing directly with the parser, a developer needs to handle the problem: - the file does not exist in the current path, so perhaps can I look for it elsewhere. - the syntax is wrong? I can ignore the file and look for an other one. But higher level classes, involving the previous one, should certainly decide to wrap lower level exceptions into unchecked ones. As a developer, when dealing with TimeScales, I don't care about IOException and, as spotted by Yannick, I'm strictly unable to do anything at this level. Another way to decide if an exception must be checked or unchecked is to consider the *contract* of the method. If the caller does not respect the contract, then we can throw an unchecked one, examples: the NullPointerException, or the design of the Iterator where the caller is expected to use hasNext() before calling next(). Hope That Helps. *************************************************************** Ce courriel (incluant ses eventuelles pieces jointes) peut contenir des informations confidentielles et/ou protegees ou dont la diffusion est restreinte. Si vous avez recu ce courriel par erreur, vous ne devez ni le copier, ni l'utiliser, ni en divulguer le contenu a quiconque. Merci d'en avertir immediatement l'expediteur et d'effacer ce courriel de votre systeme. Airbus Defence and Space et les sociétés Airbus Group declinent toute responsabilite en cas de corruption par virus, d'alteration ou de falsification de ce courriel lors de sa transmission par voie electronique. This email (including any attachments) may contain confidential and/or privileged information or information otherwise protected from disclosure. If you are not the intended recipient, please notify the sender immediately, do not copy this message or any attachments and do not use it for any purpose or disclose its content to any person, but delete this message and any attachments from your system. Airbus Defence and Space and Airbus Group companies disclaim any and all liability if this email transmission was virus corrupted, altered or falsified. --------------------------------------------------------------------- Airbus Defence and Space SAS (393 341 516 RCS Toulouse) - Capital: 29.821.072 EUR - Siege social: 31 rue des Cosmonautes, ZI du Palays, 31402 Toulouse cedex 4, France