On Fri, Sep 14, 2012 at 11:29:41AM -0700, Phil Steitz wrote:
> OK, I give up. Lets do option 2. Just warn users in the User Guide
> somewhere that our APIs are in general not null-safe and unless the
> javadoc explicitly allows nulls, they can expect NPE when passing nulls.
Thanks, Phil; we are making progress, and I hope that you'll be convinced of
that at some point.
Another decision still needs to be made.
I think that everybody now agrees that wherever an argument will be directly
used (i.e. "dereferenced") inside the body of the method[1], it is safe to
not check for null (since the JVM will throw an NPE).
But, whenever an argument is passed for _later_ use (e.g. stored by a
constructor or passed to another method), we also all expressed that it is
better to fail early if the object is supposed to be non-null. In those
cases, checks are not mandatory (since the JVM will throw NPE at some
point), but we must agree on how optional checks are to be implemented.
1. The current state of affairs was to throw "NullArgumentException"; in 4.0
this exception will become a subclass of NPE and we can continue to use
it in the same way as now (i.e. possibly providing additional localizable
error messages).
2. The alternative is to directly throw a standard NPE, but without any
message, since it wouldn't be localizable with the support of CM's
"context".
So, which of those alternatives do people want to settle on?
Regards,
Gilles
[1] As opposed to being passed on to another method.
> Phil
>
> On 9/14/12 8:46 AM, Gilles Sadowski wrote:
> > 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
> >
> >
>
>
> ---------------------------------------------------------------------
> 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
