Le 02/05/2011 17:07, Phil Steitz a écrit :
On 5/1/11 3:02 PM, Gilles Sadowski wrote:
On Sun, May 01, 2011 at 08:34:31AM -0700, Phil Steitz wrote:
On 5/1/11 6:00 AM, Gilles Sadowski wrote:
On Sun, May 01, 2011 at 12:48:14PM +0200, Gilles Sadowski wrote:
On Sat, Apr 30, 2011 at 10:53:30PM -0700, Phil Steitz wrote:
On 4/30/11 4:33 PM, Gilles Sadowski wrote:
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.
I guess I misunderstood and after really seeing the consequences in
my own code, I am going to have to ask that we reopen that
discussion - i.e., I would like us to throw IAE and other standard
exceptions where appropriate, as in this case, as we have up to and
through 2.2. I know I said before that I did not see this as worth
arguing about, but I really think this change is a bad API design
decision that will cause needless hassle and surprising RTEs for
users upgrading to the new version.
I'm astonished, and for the time being, will refrain from other comments.
There is no one single best API design. What counts most IMO is that it is
consistent and leads to clean and lean code.
The old exception factories à la
-----
MathRuntimeException.createIllegalArgumentException("Error with an argument
{0}", x);
-----
failed on all accounts.
Now, I would like to settle this issue once for all. Should all CM
exceptions inherit from the standard Java exceptions hierarchies (rooted at
IAE, UOE, NPE, AE, etc.)? Although it had been answered "no" previously, it
was not my preferred choice. I could make it "yes" now but I'd rather not
changed that back and forth again and again.'
I apologize sincerely for opening this up again. I don't think
*all* exceptions thrown by [math] should derive from standard
exceptions, other than I guess Exception itself. I do think,
however, that [math] *should* throw standard exceptions directly
when appropriate and in general favor standard exceptions. In
particular, I would prefer that we revert to the 1.0-2.2 behavior of
throwing IllegalArgumentException when preconditions are violated.
I personally have no problem with using the exception factories and
will volunteer to retrofit the code if we can agree to this change.
We agreed to provide enhanced "context-enabled" exceptions as a feature to
users. Throwing plain standard exceptions contradicts that goal.
I propose to rework the hierarchies so that MathIAE will inherit from the
standard IAE. This will solve the annoyance you would have had when
upgrading your code. [And we'll keep the "addMessage" feature together with
accessors like "getArgument" and "getMax" etc.]
The exception factories were a bad design idea (IMHO of course; plenty of
arguments given elsewhere in the last 6-8 months). One of the goals of the
exceptions refactoring was to get rid of them.
Once again, I hate to flip-flop, but this is an important and
impactful decision and I have now experienced the upgrade pain
(unexpected RTEs when upgrading) directly so would like to ask that
we revisit it.
Not "unexpected RTEs". Incompatibilities are to be expected.
Anyways, this won't happen with the new-new solution.
To be clear, my opinion is that for all of the RTEs currently
generated by MathRuntimeException.createXxxException, we should
generate and throw these exceptions directly, as appropriate, using
the factory to enable localization.
No, I like the current design; I didn't like the one you want to revert to.
Consistency implies that *all* exceptions thrown from CM must behave the
same way. I thus propose to add an interface like (maybe a better name?):
---
interface ContextedException {
void addMessage(Localizable pattern,
Object ... arguments);
void setContext(String key, Object value);
Object getContext(String key);
Set<String> getContextKeys();
String getMessage(final Locale locale);
String getMessage(final Locale locale,
final String separator);
}
And all CM exceptions will implement this interface. [Instead of
automatically inheriting the behaviour by being subclasses of
"MathRuntimeException".]
I would prefer as stated above to revert to actual RTEs per 2.x
behavior. Above would be an improvement, as at least the unexpected
RTEs at upgrade would not bite (as they did me), but I see no reason
to add this machinery which is no less complex than what we had in
2.x. Lets see what others think.
Do the above mean we would have:
public class MathIllegalArgumentException
extends IllegalArgumentException
implements ContextedException
If so, then I am OK with this.
Luc
Phil
Gilles
---------------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
