On 10/27/13 7:29 AM, Gilles wrote:
> On Sun, 27 Oct 2013 10:35:45 +0100, Luc Maisonobe wrote:
>> Le 21/10/2013 22:58, Ted Dunning a écrit :
>>> +1
>>>
>>> The overwhelming standard practice is to use a plausible
>>> exception type
>>> (such as some form of IllegalArgumentException) with a message.
>>>
>>>
>>>
>>>
>>> On Mon, Oct 21, 2013 at 5:24 PM, Phil Steitz
>>> <phil.ste...@gmail.com> wrote:
>>>
>>>> I hate to open this can of worms again, but the following is just
>>>> too painful for me to ignore. From recent mods to
>>>> BinomialConfidenceInterval javadoc:
>>>>
>>>> * @throws NumberIsTooLargeException if {@code
>>>> numberOfSuccesses
>>>>> numberOfTrials}.
>>>>
>>>> The "NumberIsTooLarge" exception adds exactly zero to what
>>>> would be
>>>> more natural - just throw MathIAE. Fortunately, the message is at
>>>> least still there in the code:
>>>>
>>>> if (numberOfSuccesses > numberOfTrials) {
>>>> throw new
>>>>
>>>>
>>>> NumberIsTooLargeException(LocalizedFormats.NUMBER_OF_SUCCESS_LARGER_THAN_POPULATION_SIZE,
>>>>
>>>> numberOfSuccesses,
>>>> numberOfTrials, true);
>>>> }
>>>>
>>>> The "NumberIsTooLarge" is ridiculous. What number? Why isn't the
>>>> second number "too small?" If we really are going insist on
>>>> defining and advertising lots of little subexceptions to
>>>> MathIAE, we
>>>> need to define appropriate ones, or just leave MathIAE. My
>>>> vote is
>>>> to just allow throwing MathIAE with a descriptive message. If we
>>>> insist on adding subexceptions for everything, in this case, we
>>>> need
>>>> something like
>>>>
>>>> SubsetSizeException
>>>>
>>>> and in another set of changes that I am about to commit that will
>>>> end up similarly mangled, I will need
>>>>
>>>> InsufficientDataException
>>>>
>>>> I would like to get full community input on this topic for once
>>>> and
>>>> for all and either add a slew of new exceptions so what we
>>>> throw is
>>>> meaningful in the context of the caller, or just allow MathIAE
>>>> to be
>>>> thrown directly.
>>>>
>>>> So please all be brief and specify your preference for one of the
>>>> two options below:
>>>>
>>>> 0) allow MathIAE to be thrown directly with an informative message
>>
>> +1 to the simple MathIAE with an informative message.
>
> _Which_ argument is illegal?
> The higher level IAE is even more "ridiculous".
> The exception class can provide a programmatic answer to that
> question;
> the text message cannot: only a human can read it.
>
> I don't see what bothers you with having an exception _type_ that
> tells
> the exact same thing as the human-readable-only test message.
> The added flexibility as been explained a lot in previous
> discussions on
> this subject. [Putting one-line general statements do not bring
> any insight
> into how to design a good exception framework... The previous
> _consensus_
> was to provide exception contexts and everything that exists now
> in order
> to be add meaningful context information as the exception travels
> up the
> stack. Unfortunately this is almost never used.]
>
> What bothers _me_ is the _out-of-band_ connection between the
> LocalizedFormats
> instance and the arguments: one has to know the text message and
> the order of
> patterns in order to instantiate the exception. On the other hand,
> a specific
> exception class type separates the meaning of the arguments
> (defined by the
> constructor arg list) from the way to display it (text message, or
> another
> way at the discretion of the application user). It also allows CM
> developers
> to change the pattern without having to modify the code that
> instantiates the
> exception.
>
> That said, I agree that exception types like
> "NumberIsTooLargeException" is
> too low level, even though it _exactly_ describes what
> precondition is being
> tested.
> You could probably recover from the archive that it was a
> compromise aimed
> at reducing to the minimum the number of exception types (while
> retaining
> the minimum of the functionality which I exposed, again, in the
> previous
> paragraph).
>
> In conclusion, if the user is only interested in the text message,
> it is
> there, whatever the way (flexible or not) we choose to display it;
> if he
> wants to catch a higher level type (i.e. "IllegalArgumentException"),
> this is also possible, of course.
>
> Because of (or thanks to[1]) the required localization, the only
> way to
> improve both functionality and meaningfulness of the reported failure
> is to add more types.
I am OK adding more types. I think the two I suggested above,
SubsetSizeException and InsufficientDataException would be useful
(along with precise messages and context data) in quite a few places
to replace the less meaningful low-level ones. Are you both OK with
me adding these and starting the move to use them where they make sense?
Phil
>
> If you don't want to look into this further[2], at least do not
> remove
> whatever little exists that could serve as the basis for further
> work.
> [Just look at the amount of duplication in the error text patterns.]
>
>
> Regards,
> Gilles
>
> [1] Since the requirement made me aware that something more elaborate
> than a String pattern was needed to ultimately create the
> displayed
> text message.
> [2] There are other issues about exceptions which are being blocked
> (because they are also deemed "too trivial"?).
>
>
>> best regards,
>> Luc
>>
>>
>>>>
>>>> 1) define caller-meaningful exceptions for situations such as
>>>> insufficient data, invalid subset size, invalid probability,
>>>> invalid
>>>> interval, ...
>>>>
>>>> I would much prefer 0), but if consensus is 1), I will start
>>>> adding
>>>> exceptions so what we throw is meaningful and open a ticket to
>>>> clean
>>>> up for 4.0.
>>>>
>>>> Phil
>>>>
>
>
> ---------------------------------------------------------------------
> 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
