[
https://issues.apache.org/jira/browse/MATH-487?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12983931#action_12983931
]
Gilles commented on MATH-487:
-----------------------------
What bothers me is that I don't see any added value to such seemingly
"high-level" exceptions. Primarily because the first rule is that the exception
should be thrown as close as possible to the source of the problem. And I think
that the exception should describe the local problem, and not try to convey
what a local failure means for the caller. In this case, we have
# The callee says: "Hey, this quantity is infinite!"
# The caller figures out: "The ContinuedFraction has a divergence problem."
I think that, CM being a low-level component, it is fine to throw low-level
exceptions. The callee encounters a problem and signals it faithfully to the
caller; the caller is then free to pass on the information and change (or wrap)
it into a higher level type such as "ConvergenceException". It can do so
because the CM code must have a comment stating:
{noformat}
/**
* @throws NotFiniteNumberException when the continued fraction fails to
converge.
*/
{noformat}
In the above Javadoc excerpt, the type provides a description of the (local)
problem and the comment describes the cause of the problem. They are
complementary while, if using a "ConvergenceException" type, the comment
becomes redundant.
Moreover, I think that mixing low-level ("MaxCountExceededException") and
high-level abstractions ("ConvergenceException") does not make for particularly
clear code (i.e. when to use which level?). Again, it's not that I don't like
the idea of high-level exceptions, it's just that I don't see how to make them
coexist with low-level ones.
Creating an unchecked "ConvergenceException" and is certainly very easy, but I
don't see it as so clear-cut what the best solution is.
> 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.
