You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Tharaka De Silva <th...@gmail.com> on 2017/06/07 22:00:46 UTC

NUMBERS-40: Review exception usage in package "o.a.c.numbers.gamma"

Hello everyone,

I am new to the ASF community and decided to grab something easy to
attempt. I decided to take a shot at:
https://issues.apache.org/jira/browse/NUMBERS-40.

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.

What do you guys think?

-- 
Sincerely,

Tharaka De Silva

Re: NUMBERS-40: Review exception usage in package "o.a.c.numbers.gamma"

Posted by Gilles <gi...@harfang.homelinux.org>.
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 <gi...@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


Re: NUMBERS-40: Review exception usage in package "o.a.c.numbers.gamma"

Posted by Tharaka De Silva <th...@gmail.com>.
Thank you for the response!

On Thu, Jun 8, 2017 at 5:13 AM, Gilles <gi...@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.


>
> 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?


> 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.


>
>
> 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
>
>


-- 
Sincerely,

Tharaka De Silva

Re: NUMBERS-40: Review exception usage in package "o.a.c.numbers.gamma"

Posted by Gilles <gi...@harfang.homelinux.org>.
Hi.

On Wed, 7 Jun 2017 17:00:46 -0500, Tharaka De Silva wrote:
> Hello everyone,
>
> I am new to the ASF community

Welcome.

> 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. ;-)

>
> 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.

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).

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.


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