[
https://issues.apache.org/jira/browse/MATH-487?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12984700#action_12984700
]
Gilles commented on MATH-487:
-----------------------------
I know that the different criteria (tolerances, number of iterations, etc.) are
somehow linked and that various combinations can lead to a convergence failure.
It might be nice to have a class that encompasses all the ways that can lead to
such a failure. We don't have one and thus you want to stick with an empty
shell {{ConvergenceException}}. This step, instead of improving the situation
(e.g. reducing the number of redundant messages) can only make it worse because
developers will start use this half-broken place-holder and since there is no
state in that class nor design proposal to figure out one, they will just add
new messages to convey the exact problem they encounter (and will sometimes
create new messages even if an equivalent one already exists... Doesn't that
sound familiar?). Furthermore, I'm quite sure that some of those messages will
be low-level and others will be high-level (as what is important or
implementation detail will vary according to the developer, as shown even in
this very short code that is the {{ContinuedFraction}} case); this will become
a mess.
The "unfinished" {{ConvergenceException}} is not a compromise because
# It departs from the current work of simplifying the exceptions generation and
handling which is reporting the direct cause of the failure, and not try to
outguess the caller.
# I'm afraid that the ideal
("one-size-fits-all-combinations-of-convergence-criteria")
{{ConvergenceException}} is so complicated that, if not downward impossible to
achieve, it will become a sort of framework of its own . At least, it will take
a long time before anyone comes up with a working design, leaving ample time to
build up the mess I refer to above. That kind of mess is what led to issue
MATH-195, and I'm sorry I have to say your compromise paves the way back to
square one.
I'm in favour of the "KISS" principle: Let's stick with what is known to be
useful (i.e. reporting the problem as CM sees it); let's not base current
coding on hope for functionality that nobody knows how to implement.
If the design for high-level exceptions arises some day, it will be relatively
easy to wrap the simple, local, low-level exceptions into these new ones.
The current simplification turns out to be quite some work; if we hope that CM
will grow, but don't stick to simple rules, we run the risk that it becomes
inconsistent and unmanageable.
> 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.
