commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gary Gregory <garydgreg...@gmail.com>
Subject Re: [math] UnexpectedNegativeIntegerException
Date Sun, 02 Sep 2012 11:52:42 GMT
Jumping in the middle ... Just about naming:

The "unexpected" in the name is redundant IMO because it is an
"exception". It also does not need to be distinguished from an
"expected" exception. NIE would work or IllegalNIE if you want to be
more wordy. On the other extreme, there is also the existing
IleegalArgumentException, but it sounds like you've gone down the road
of super specific exceptions.

Gary

On Sep 2, 2012, at 7:26, Gilles Sadowski <gilles@harfang.homelinux.org> wrote:

> On Sun, Sep 02, 2012 at 12:24:58PM +0200, Luc Maisonobe wrote:
>> Le 02/09/2012 00:16, Gilles Sadowski a écrit :
>>> Hello Luc.
>>
>> Hi Gilles,
>>
>>>
>>>>>
>>>>>>>> [...]
>>>>>>>> I encountered this need in two different cases. The first
one was to
>>>>>>>> identify very precisely an error type, even with finer granularity
than
>>>>>>>> exception type. Parsing the error message to recognize the
exception is
>>>>>>>> evil, checking the enumerate used in the pattern is less
evil. The
>>>>>>>> second case was when I needed to create a more elaborate
message by
>>>>>>>> combining some information provided by the caller, and some
information
>>>>>>>> extracted from the exception. Here again, parsing is evil
but getting
>>>>>>>> the parameters is fine.
>>>>>>>
>>>>>>> Maybe you missed my point (same as above), as it applies here
too: You can
>>>>>>> get the parameters through the accessors (of the specific exception
types).
>>>>>>> We created the "context" so that additional parameters can be
set and
>>>>>>> retrieved ("key/value" pairs). I still do not understand why
one should
>>>>>>> resort to extract something from the pattern.
>>>>>>> [The pattern is unfortunately "public" whereas it should be an
>>>>>>> "implementation detail".]
>>>>>>
>>>>>> I don't "extract" something from the pattern, I just check if it
is a
>>>>>> known and expected enumerate I want to handle specifically or something
>>>>>> else.
>>>>>
>>>>> Maybe I should see the actual code before we go on discussing this. Of
>>>>> course you are free to do whatever the API of CM lets you do. I just
have
>>>>> the feeling that I would do it otherwise... :-}
>>>>
>>>> Here is an example I encountered two minutes ago, while working on
>>>> adding the exception throws statements in the ode package.
>>>>
>>>> The computeParameterJacobian may throw a MathIllegalArgumentException
>>>> (in the current subversion repository, I have seen it throws an
>>>> IllegalArgumentException, which is wrong). In the following piece of
>>>> code, I need to check the exception I get is really a
>>>> LocalizedFormats.UNKNOWN_PARAMETER or something else:
>>>>
>>>>
>>>>  delayedExcpetion = null;
>>>>  for (ParameterJacobianProvider provider: jacobianProviders) {
>>>>
>>>>    try {
>>>>
>>>>      provider.computeParameterJacobian(...);
>>>>      return;
>>>>
>>>>    } catch (MathIllegalArgumentException miae) {
>>>>      List<Localizable> patterns = miae.getContext().getPatterns();
>>>>      if (patterns.contains(LocalizedFormats.UNKNOWN_PARAMETER)) {
>>>>        // temporarily ignore, until we have checked all providers
>>>>        delayedException = miae;
>>>>      } else {
>>>>        // this is another problem, report it
>>>>        throw miae;
>>>>      }
>>>>    }
>>>>
>>>>  }
>>>>
>>>>  if (delayedException != null) {
>>>>    // none of the providers did handle the parameters
>>>>    throw miae;
>>>>  }
>>>>
>>>> Here, I only use only the pattern in the check, sometimes I need to also
>>>> check the parameters (for example here I could use the name of the
>>>> parameter). Note that the exception thrown here is a high level
>>>> MathIllegalArgumentException, not a specialized exception like
>>>> NumberIsTooLarge which does have specialized getters like getMax.
>>>>
>>>> So for this kind of use, I need getters in the context class
>>>> (getPatterns and getParameters). The alternative would be to create
>>>> specialized exceptions for every single LocalizedFormats we have, which
>>>> is clearly too much.
>>>
>>> What can I say? :-)  IMO, for every such use, there should indeed be a
>>> specific exception class. It's really a pity that a user (you) should do
>>> this gymnastics because CM developers (Oh, that'd be you too! ;-) didn't
>>> want to have a specific type for conveying a specific problem.
>>
>> I agree that inside [math], there are other ways to do it. The problem
>> occurs also (and in fact mainly) in client applications that only use
>> [math]. They cannot change the exceptions thrown.
>
> I hold the opposite view: Inside CM code we could get away with "dirty
> tricks" because they are impementation details, which we can change
> transparently. However, your example demontrates that CM does not do it the
> right way (since the user must resort to "non-standard" practice in order
> to get the information he needs): The logical conclusion is what I
> explained (avoid to throw general exceptions when the user might need a
> specific one). [That is, the user can legitimately file a request for
> improvement so that CM developers will change what is currently wrong.]
>
> What the user does with the current state of the library is a workaround!
> Please look at it this way: Why should anyone concerned with math algorithms
> have to deal with something called "LocalizedFormats" to control his code
> flow? [The name says it all: the "enum" is aimed at localizing text
> messages; any other use is just a hack.]
>
>
> Best,
> Gilles
>
>>
>>>
>>> "MathIllegalArgumentException" is a high-level abstraction for the usage of
>>> _users_ with the sole purpose that they do not have to duplicate code when
>>> they just want to catch a whole slew of exceptions in one go (by the way,
>>> this is exactly the same rationale as for having a "MathRuntimeException");
>>> developers should not throw this exception but rather a specific one that
>>> describes the exact problem at hand.
>>> If the exception is very "local" to the code, like in the above, you could
>>> have it defined in the package (or as an inner class). With this, your user
>>> code could become
>>>
>>> ---CUT---
>>>   delayedException = null;
>>>   for (ParameterJacobianProvider provider: jacobianProviders) {
>>>     try {
>>>       provider.computeParameterJacobian(...);
>>>       return;
>>>     } catch (org.apache.commons.math3.ode.UnknownParameterException miae) {
>>>       // temporarily ignore, until we have checked all providers
>>>       delayedException = miae;
>>>     }
>>>   }
>>>
>>>   if (delayedException != null) {
>>>     // none of the providers did handle the parameters
>>>     throw miae;
>>>   }
>>> ---CUT---
>>>
>>> Simpler (for you, as a user), cleaner (no "if-else", no need to dig into the
>>> internals of CM), more robust (what if I have it my way and we dump
>>> "LocalizedFormats"? ;-D), more efficient (no catching of the wrong
>>> exception, no extracting of the patterns, no list search), more concise
>>> (hence easier to understand).
>>
>> In this case, yes, it's easier. This is not possible when you are not a
>> [math] developer or when the rest of the [math] community opposes to the
>> change.
>>
>>>
>>> [Personally, I find it extremely ugly and brittle to rely on a "String" to
>>
>> It's not a "String"! It is an enumerate and in fact a complete class
>> which also supports being translated. enumerates "are" designed to be
>> compared to each other using simple "==" operator.
>>
>>> convey that a particular condition has occurred. I'm pretty sure that _you_
>>> can do that because you are the one who wrote the CM code (for that reason,
>>> you know what it means). No other CM user could have written a code like you
>>> did above (at least not without having read the source code).]
>>
>> Yes, of course, and it is the reason why I want to handle this exception
>> in a specific way. I need to identify it.
>>
>> There is also another use case: I get the exception and simply want to
>> improve the message. In order to do this in a localizable way, I need
>> access to the pattern (this time as a localizable, not as an enumerate)
>> and to the parameters so I can combine them with upper level
>> information. And once again, when you are not a [math] developer, you
>> cannot simply use the exception type since there is no bijection between
>> the exception type and the problems identified.
>>
>>>
>>> Thanks for the discussion; I hope I showed you that there is indeed another
>>> way to do it.
>>
>> In this case, yes, in the general case, no.
>>
>>>
>>> Please note that I don't say that there must be one exception for each
>>> "enum" element. But it's not because there are too any of them.
>>
>> This is exactly my point. If I want to identify (first use case) or
>> reuse (second use case) the enum, I need it.
>>
>> I really don't see where the problem is in adding two getters in the
>> ExceptionContect class. There are use cases for these getters and not
>> providing them would force people who need them to go to really dirty
>> tricks to circumvent the fact we don't provide them.
>>
>> Luc
>>
>>> There should be an exception for each "broad" class of problems ("out of
>>> range", "no data", "not positive", etc. plus more "local" ones, once their
>>> usefulness as _type_ is identified, like in your example).
>>> The _displayed_ message can be enhanced by adding contextual information
>>> when instantiating the exception (like the "standard deviation" is "not
>>> positive") but the information meant for display should not be used as a
>>> substitute for a proper type.
>>> There are indeed too many "enum" elements; many of them could just be
>>> deleted. [Some time ago, I had made some steps toward a big cleanup.]
>>>
>>>
>>> Best,
>>> Gilles
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message