[jira] Commented: (MATH-487) Deprecate "ConvergenceException" in MATH_2_X and remove it in trunk

["Phil Steitz (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/MATH-487?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12983737#action_12983737 ] 

Phil Steitz commented on MATH-487:
----------------------------------

I am fine with deprecating the checked exception.

I want to make sure, though that we do not lose context information in the exception instances that replace it.  The first example above loses information, IMO.  The fact that scale is at that point not strictly positive is an implementation artifact, generally useless to the caller.  What is actually going on is that the fraction is diverging to infinity (different from diverging to NaN) and *that* is what should be reported.

Honestly, I think that ConvergenceException is not a bad abstraction.  Perhaps just make it unchecked?  Or define subclasses for more specific convergence failures?

> Deprecate "ConvergenceException" in MATH_2_X and remove it in trunk
> -------------------------------------------------------------------
>
>                 Key: MATH-487
>                 URL: https://issues.apache.org/jira/browse/MATH-487
>             Project: Commons Math
>          Issue Type: Improvement
>            Reporter: Gilles
>            Assignee: Gilles
>            Priority: Minor
>             Fix For: 2.2, 3.0
>
>
> The checked "ConvergenceException" should be deprecated.
> An example usage is in class {{ContinuedFraction}} (package {{util}}), at line 153:
> {noformat}
>   if (scale <= 0) {  // Can't scale
>     throw new ConvergenceException(LocalizedFormats.CONTINUED_FRACTION_INFINITY_DIVERGENCE, x);
>   }
> {noformat}
> I think that it should be replaced by a more specific (and unchecked) exception that reflects the exact low-level problem:
> {noformat}
>   if (scale <= 0) {  // Can't scale
>     throw new NotStrictlypositiveException(scale);
>   }
> {noformat}
> A few lines below that, there is:
> {noformat}
>   if (infinite) {
>     // Scaling failed
>     throw new ConvergenceException(LocalizedFormats.CONTINUED_FRACTION_INFINITY_DIVERGENCE, x);
>   }
> {noformat}
> So it seems that it is not necessary to throw an exception at the place where the test on "scale" fails; instead we could have:
> {noformat}
>   infinite = true;
>   if (scale <= 0) {  // Can't scale
>     break;
>   }
> {noformat}
> and let the check on "infinite" throw the exception:
> {noformat}
>   if (infinite) {
>     // Scaling failed
>     throw new NotFiniteNumberException(LocalizedFormats.CONTINUED_FRACTION_DIVERGENCE,
>                          Double.POSITIVE_INFINITY, x);
>   }
> {noformat}
> As shown in the above excerpt, we could also replace two {{enum}}:
> * CONTINUED_FRACTION_INFINITY_DIVERGENCE
> * CONTINUED_FRACTION_NAN_DIVERGENCE
> with a single one:
> * CONTINUED_FRACTION_DIVERGENCE
> because the other bit of information (infinity vs NaN) is already given by the first parameter of the message.
> What do you think of these changes?

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.