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".]
Gilles
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
