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.
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
