On Sat, Apr 30, 2011 at 09:10:08AM -0700, Phil Steitz wrote:
> Converting some of my code to use trunk, I discovered that the
> binomialCoefficient methods no longer throw IllegalArgumentException
> when parameters are invalid.
The consensus was a singly rooted hierarchy ("MathRuntimeException").
The advantage being commonly agreed on was to offer the "map" functionality
for adding messages and context information.
> The javadoc asserts that
> MathIllegalArgumentException will be thrown in these cases, but that
> is not correct,
I don't understand; the code for "checkBinomial" can throw
"NumberIsTooLargeException" or "NotPositiveException" and they are
subclasses of "MathIllegalArgumentException".
> since what is actually thrown now can differ
> depending on the parameter problem
That's a feature, naturally: Different problem, different exception.
> and the resulting exceptions are
> neither standard IAEs nor descendents of MathIAE.
>From what I see, they are descendents of MathIAE.
> I have patched
> the code to return a standard IAE (with localized message). Per
> discussion in [1] it looks like we were close to consensus to favor
> standard exceptions and in this case,
No, that thread was discussing
throwing standard "NullPointerException"
vs
throwing a CM-specific "NullArgumentException" (subtype of MathIAE)
vs
not checking for null pointer at all.
[I don't think that a consensus has been reached on that issue.]
For all the other cases of invalid parameters passed to methods, it had
been settled already that "MathIllegalArgumentException" (or subclasses
thereof) would been thrown.
> I would much rather return a
> standard IAE with meaningful error message rather than a
> non-standard RTE (with exactly the same error message and generally
> confusing type - e.g. "NumberIsTooSmall" when n, k parameters are
> not in the right order) and keep the javadoc simple. Otherwise, the
> main method javadoc has to be rewritten to conform to what the code
> now does.
The Javadoc "@throws" tag is not incorrect:
-----
* @throws MathIllegalArgumentException if preconditions are not met
-----
But it is not as precise as it could (by mentioning the types actually
thrown by "checkBinomial").
The main description is indeed a remnant of the old behaviour and it is yet
another proof that it is not good documentation practise to repeat the
(supposedly) same information several times.
Documentation for methods "binomialCoefficientDouble" and
"binomialCoefficientLog" also refer to the old behaviour and must be
updated.
> Are there any objections to this change?
Unless I didn't understand the problem, yes.
-1 to revert to a mix of standard and CM-specific exceptions.
Gilles
> Phil
>
> [1] *http://s.apache.org/nr*
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
