Mailing-List: contact dev-help@commons.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Commons Developers List" <dev@commons.apache.org>
Received-SPF: pass (athena.apache.org: local policy)
Date: Sun, 2 Sep 2012 13:25:59 +0200
From: Gilles Sadowski <gilles@harfang.homelinux.org>
To: dev@commons.apache.org
Subject: Re: [math] UnexpectedNegativeIntegerException
Message-ID: <20120902112559.GR24856@dusk.harfang.homelinux.org>
Mail-Followup-To: dev@commons.apache.org
References: <503A5FF8.1030408@gmail.com>
 <503CBB41.7050005@free.fr>
 <20120828140434.GZ24856@dusk.harfang.homelinux.org>
 <503D0501.5040003@free.fr>
 <20120828225125.GB24856@dusk.harfang.homelinux.org>
 <503DC55B.70903@free.fr>
 <20120829200215.GD24856@dusk.harfang.homelinux.org>
 <504205B8.3020508@free.fr>
 <20120901221624.GQ24856@dusk.harfang.homelinux.org>
 <504333FA.5000704@spaceroots.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <504333FA.5000704@spaceroots.org>
User-Agent: Mutt/1.5.21 (2010-09-15)
X-DCC-scarlet.be-Metrics: sif 20001; Body=1 Fuz1=1 Fuz2=1
X-Virus-Scanned: clamav-milter 0.97.1-exp at sif
X-Virus-Status: Clean
X-Virus-Checked: Checked by ClamAV on apache.org

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