On Fri, Sep 14, 2012 at 07:44:08AM -0700, Phil Steitz wrote: > On 9/14/12 4:28 AM, Gilles Sadowski wrote: > > On Fri, Sep 14, 2012 at 10:41:50AM +0200, Luc Maisonobe wrote: > >> Le 14/09/2012 08:46, Sébastien Brisard a écrit : > >>> Hi Phil > >>> > >>>>> Back to square one, with 3 fully consistent alternatives: > >>>>> 1. CM to always check for null? Then "NullArgumentException" > >>>>> inheriting from > >>>>> "MathIllegalArgumentException" is fine because we promise to the > >>>>> user that > >>>>> no NPE will ever propagate through the CM layer. [Breaking that > >>>>> promise > >>>>> is a CM bug.] > >>>>> 2. CM to never check for null? Then we delete class > >>>>> "NullArgumentException". > >>>>> Users are warned by the general policy: "Do not pass null unless it > >>>>> is > >>>>> explicitly documented to be allowed." A bug will lead to the JVM > >>>>> raising > >>>>> a NPE. > >>>>> 3. CM to sometimes check for null? Then "NullArgumentException" should > >>>>> inherit from "NullPointerException" because the user will sometimes > >>>>> see > >>>>> "NullArgumentException" (when CM checks) and sometimes NPE (when CM > >>>>> does > >>>>> not check) and both should thus belong to the same hierarchy (from > >>>>> the > >>>>> user's point-of-view, there is no reason to handle them in different > >>>>> ways since it's the exact same bug). > >>>>> For the user, the consequence will be similar to alternative 2, with > >>>>> sometimes more information about the failure and sometimes > >>>>> (marginally) > >>>>> earlier detection. > >>>> As stated above, my preference is > >>>> > >>>> 4. CM APIs *may* disallow nulls explicitly. When that is done, the > >>>> implementations *should* check parameters (as they *should* check > >>>> all other stated preconditions) and when preconditions are violated, > >>>> a MathIllegalArgumentException is thrown. I don't care whether or > >>>> not we keep NAE. If we drop it, we should make sure whatever > >>>> exception messages we used to use to indicate illegal null arguments > >>>> are still there. > >>>> > >>>> Phil > >>>> > >>> I like this option better than 3. So, I'll change my "vote" to option > >>> #2, and possibly option #4 as we will never agree on option #2. > > Why is it better to throw MathIllegalArgumentException rather than NPE? > > Because, as I have repeated numerous times, that is the appropriate > exception to throw when API preconditions are violated. That is why > IAE exists. The class javadoc comment for IAE is nice and succinct > (we could learn from it ;)
IllegalArgumentException: --- Thrown to indicate that a method has been passed an illegal or inappropriate argument. --- Did you read the class Javadoc for NPE? Here is the last sentence pasted again: --- Applications should throw instances of this class to indicate other illegal uses of the null object. --- > You could ask the same question about > ArrayIndexOutOfBounds. Why not throw that instead of IAE when bad > indices are provided? Does CM check _array_ index? No. Surprised? An "ArrayRealVector" is _not_ an array, it is the representation of the vector concept. That it is a backed by a Java array is an implemtation detail that should not surface to the API. Hence the choice to not use the low-level ArrayIndexOutOfBoundsException can be given a rationale. CM checks that you access valid entries of the high-level "mathematical" object. And we do that for _all_ such accesses and never (or it is a CM bug) let propagate an exception that reveals an underlying Java array. [This was one of my proposals too (alternative 1). However it imposes a unnecessary burden of duplicating (CM and JVM) every null check just for the sake of hiding NPE.] As indicated in a previous post, "null" is not a "RealVector", it is not an "Object", it is not a reference, it is just the value of an unitialized pointer. > The difference is that instead of letting > the precondition failure go undetected and the runtime exception > propagate, APIs with explicit preconditions and parameter checking > in place raise the exception at method activation time. Could you please stop mentioning this, since my proposal (alternative 3) allows for early detection? The difference is that you want to use MathIAE, where everyone else uses NPE. > In that > context, what is appropriate is IAE. You are stuck with a "name", as if everything that is "illegal" must be signalled with an exception that contains the string "Illegal" in its name. If that is so, I propose to rename "NullArgumentException" to "IllegalNullArgumentException". Gilles > > Phil > > > >> My preferences are 4 or 3 with same preference level and 2 as a fallback > >> choice. For each option, I find the arguments are good, it's only a > >> matter or preference. > > Did everyone consider that Phil's alternative implies a behaviour that > > departs from the standard practice, a.o. > > * JDK (cf. quote in a previous post), > > * other libraries, e.g. Guava (cf. quote in a previous post), and > > * Java experts, e.g. Joshua Bloch[1] > > of throwing NPE when they encounter "null"? > > Doesn't anyone care about having a _reason_ for CM to behave differently? > > > > Don't you care that users will see different exceptions when the same > > problem is detected? > > > > Do you really like a policy that mandates different behaviours when "null" > > is disallowed implicitly vs explicitly (throwing NPE vs throwing > > MathRuntimeException)? [Pardon me, but IMHO this is nonsense.] > > > > After piling up arguments (_none_ of which have been addressed), I'm sorry > > to read that it would only be a "matter of preference"! [I should start a > > career as a writer if I'm able to express "preference" in so many words...] > > Actually, it's a matter of consistency ("same problem, same exception"). > > If nobody cares about improving CM's consistency, then indeed most of the > > discussions in which I take part are pointless. > > > > What is wrong with throwing NPE? [Alternative 3 is a compromise: it allows > > CM to fail as early as possible (which I thought was Phil's main point, as > > it was his only one, apart from "preference"), while being consistent ("same > > problem, same exception"), internally, externally, implicitly and > > explicitly. Alternative 4 is inconsistent ("same problem, different > > exceptions"); it's a truth, not a matter of taste.] > > > > Rather then being willingly stuck in a deadlock because only the weakest > > argument ("preference") is taken into account, wouldn't it be more > > reasonable to count all the arguments? > > > > > > Gilles > > > > [1] "Effective Java" (second edition). Excerpt from "Item 60": > > If a caller passes null in some parameter for which null values are > > prohibited, convention dictates that NullPointerException be thrown > > rather than IllegalArgumentException. > > > >> Luc > >> > >>> Best regards, > >>> Sébastien > >>>>> Your alternative (sometimes check, sometimes not) is not fully > >>>>> consistent: > >>>>> * for the user: "In case of null reference, will I get a > >>>>> MathRuntimeException > >>>>> or a NPE?" > >>>>> * for the CM developer: "In which cases do I need to check for null?" > >>>>> > >>>>> Of course, I would reconsider if you could provide an actual example > >>>>> that > >>>>> would fail with all three alternatives which I suggested. If not, then > >>>>> it seems obvious that your alternative presents two defects that don't > >>>>> exist > >>>>> in any of those three. > >>>>> > >>>>> > >>>>> Gilles > >>>>> > >>>>>>>> [...] what is different about null arguments at the point of > >>>>>>>> method activation in an API that explicitly disallows them. > >>>>>>> The difference is that there is no need to tell the user what the > >>>>>>> problem > >>>>>>> is because the raised exception says it all: "You tried to use a null > >>>>>>> reference." [As said above, the only issue is _when_ the exception is > >>>>>>> triggered.] > >>>>>>> > >>>>>>> The policy mandates what is globally valid, e.g.: > >>>>>>> "If not specified otherwise, "null" is not allowed as an argument." > >>>>>>> Then, a user cannot complain about a NPE being propagated when he > >>>>>>> passed an > >>>>>>> invalid (null) argument. > >>>>>>> > >>>>>>> Finally, the case for "null" is also slightly peculiar (given the > >>>>>>> above) > >>>>>>> that it should not simply be bundled with the rationale "Fail early": > >>>>>>> Indeed > >>>>>>> "null" references always fail early (i.e. as soon as they are used). > >>>>>>> Deferring the check until it is done by the JVM will never entails > >>>>>>> the code > >>>>>>> to produce a wrong result _because_ of that null reference (it will > >>>>>>> just > >>>>>>> fail in the predictable way: NPE).[1] > >>>>>>> > >>>>>>> > >>>>>>> Gilles > >>>>>>> > >>>>>>> [1] Unlike in C, where an unitialized pointer would not necessarily > >>>>>>> crash > >>>>>>> the program, but _could_ make it behave erratically (including > >>>>>>> produce > >>>>>>> wrong results in a stealth way). > >>>>>>> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org