commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gilles <gil...@harfang.homelinux.org>
Subject Re: NUMBERS-40: Review exception usage in package "o.a.c.numbers.gamma"
Date Sat, 10 Jun 2017 13:57:47 GMT
Hi.

On Thu, 8 Jun 2017 19:38:55 -0500, Tharaka De Silva wrote:
> Thank you for the response!
>
> On Thu, Jun 8, 2017 at 5:13 AM, Gilles <gilles@harfang.homelinux.org> 
> wrote:
>
>> Hi.
>>
>> On Wed, 7 Jun 2017 17:00:46 -0500, Tharaka De Silva wrote:
>>
>>> Hello everyone,
>>>
>>> I am new to the ASF community
>>>
>>
>> Welcome.
>
>
> Thank you!
>
>
>>
>>
>> and decided to grab something easy to
>>> attempt. I decided to take a shot at:
>>> https://issues.apache.org/jira/browse/NUMBERS-40.
>>>
>>
>> Easy to change is not always similar to easy to decide what
>> changes to perform. ;-)
>>
>>
> I did not think of it that way. It makes sense.
>
>
>>
>>> The rationale of implementing this design would be this:
>>> GammaException is currently a subclass of IllegalArgumentException 
>>> but the
>>> reason for an argument to be invalid would be because it is 
>>> arithmetically
>>> impossible hence why it should be an ArithmeticException rather 
>>> than a
>>> IllegalArgumentException.
>>>
>>
>> In quite a few cases, it is actually _not_ "arithmetically 
>> impossible",
>> it is a limitation of the implementation.
>>
>
> I was looking at it and the one I saw was for negative log and I made 
> my
> assumptions from that.

In effect, I raised the issue to make sure that the way to handle
(input) errors is consistent throughout the component.

The choice is even larger; for example, in the "log" case, do we
want to raise an exception on negative input, or return NaN (as
"Math.log" does)?
Which is more useful/safer/more standard?

>>
>> The JIRA report asks whether it is possible to use a single 
>> exception
>> type (currently "GammaException") for all programming errors, given 
>> that
>> the base class of all errors _cannot_ be "ArithmeticException" (as 
>> the
>> above explains).
>>
>>
> So you think that leaving it as a subclass of 
> IllegalArgumentException
> makes more sense?

When the Javadoc states that a method should not be called with
some selection of the argument(s), then yes, I do.  [A priori; but
another rationale could come out from answering the above questions,
comparing with how those situations are handled in class "Complex".]

>> As an aside, in the unit tests, the use of the exception's base 
>> (JDK)
>> class in the "expected" clause is intentional as the unit tests are
>> mainly supposed to  check the public API (and "GammaException" is
>> package-private).
>> In "Commons Numbers", the idea would be to have a most simple 
>> exception
>> handling (throwing only JDK exceptions) since it is expected (TBC) 
>> that
>> all of them result from incorrect usage or bugs in the library.
>>
>
> Yeah, I didn't think of it this way. The unit tests probably should
> probably be modified to assert for the JDK exceptions.

We must first decide whether the public API should contain specific
exceptions; the question is whether the caller can do more with a
specific type than with a JDK exception.
E.g. if use-cases only involve reading the stack trace after a
program crash, in order to fix the bug, then the answer is "no".


Regards,
Gilles

>>
>>
>>> What do you guys think?
>>>
>>
>>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message