[
https://issues.apache.org/jira/browse/MATH-487?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12984502#action_12984502
]
Gilles commented on MATH-487:
-----------------------------
I'm not an expert in numerical analysis, and I certainly don't want to imply
that the concept of convergence is not important. I still can say that in the
current {{ConvergenceException}}, the link to this concept is too vague. You
want to convey a high-level notion (convergence problem when evaluating a
continued fraction at "x") but you don't say what happened exactly. Which I
think doesn't make sense at the CM level; in all other cases where exceptions
are thrown, we explain what, exactly, went wrong (e.g. how the precondition was
violated). Here, should we be content to say that something called
"convergence" went wrong?
If the problem in the {{evaluate}} method is a numerical one, then maybe that
the appropriate exception type is more akin to {{MathArithmeticException}}.
Moreover, I think that if the potential infinities and NaNs arise from
numerical problems, it should not be considered as an implementation detail but
reported as such.
An empty shell is not fine; it is an indication that something is missing.
{quote}
[...] using a too general exception for too many different things is bad [...]
{quote}
I totally agree, and that's why I'm saying that {{ConvergenceException}}
without any state is useless: What do you expect to be able to do by catching
specifically a stateless {{ConvergenceException}} that you wouldn't be able to
do by catching, say, a {{MathRuntimeException}}?
> 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.
