Let me stir the pot:
At a fundamental level I agree that a required *argument* should throw an
IllegalArgumentException when null is supplied. However, ISTR the decision
to do otherwise in [lang] having been discussed on-list in the
not-so-distant past, and the result of that discussion being that NPE is
usually the result in the core JDK classes. So I wouldn't characterize the
situation as "[lang] *just happens* to throw NPE." Now, [lang] is in a
slightly unique situation as its stated mission is to complement Java SE,
so it could be argued that [lang] is under greater pressure to conform for
that reason. But my personal preference, in light of the standing decision
with [lang], would be for consistency throughout Commons components
*despite* the fact that at a purely semantic level I agree with the
arguments in favor of IllegalArgumentException.
To summarize, +1 for NullPointerException from me.
Matt
On Fri, Aug 30, 2013 at 9:36 AM, Benedikt Ritter <brit...@apache.org> wrote:
> 2013/8/30 sebb <seb...@gmail.com>
>
> > On 30 August 2013 15:19, Benedikt Ritter <brit...@apache.org> wrote:
> > > I've removed the generics in r1518974, thanks for spotting that sebb.
> > >
> > > Regarding the exception to throw I'm indifferent. The important thing
> is
> > to
> > > fail early.
> > >
> >
> > ... and with a helpful message.
> >
> > > I personally thing that NPE should be reserved for signaling that some
> > code
> > > tried to invoke a method on a null reference.
> >
> > +1
> >
> > > In our case null is illegal because we know that some code later on
> would
> > > break if we accept a null reference. So IllegalStateException makes
> > sense.
> >
> > s/IllegalStateException /IllegalArgumentException/
> >
> > +1
> >
>
> Sorry, I meant IAE of corse.
>
>
> >
> > > Imaging having a method that accepts an int and only positive ints are
> > > valid. You would throw an IllegalArgumentException if a negative int is
> > > passed to that method and not a NegativeIntegerException (or what
> ever).
> > > But James has a point. If [LANG] uses NPE maybe we should stick to
> this.
> >
> > I don't think we have to do the same as LANG, so long as the Javadoc is
> > clear.
> >
> > > Feel free to change it. I'll leave it for now since there doesn't seem
> to
> > > be consensus?!
> >
> > Unless there are other reasons than "LANG happens to use NPE" I think
> > we should stick with IAE, as it more clearly indicates the the problem
> > is not within the method throwing it.
> >
> > The problem with using NPE to flag parameter errors is that it
> > confuses the user as to the likely cause.
> >
> > Leave NPEs to the runtime system.
> >
>
> agreed.
>
>
> >
> > > Benedikt
> > >
> > >
> > > 2013/8/30 James Carman <ja...@carmanconsulting.com>
> > >
> > >> Well, the problem with using NPE is that we as Java developers have
> > >> learned through the years that NPE typically is an "oh crap"
> > >> situation, where something is terribly wrong (we hate seeing those).
> > >> Thus, our users may have knee-jerk reactions and not even know to
> > >> inspect the message for the real cause. IAE is less alarming, IMHO.
> > >> Just my $0.02, but we've been doing it that way for a long time in
> > >> [lang], so be it.
> > >>
> > >>
> > >> On Fri, Aug 30, 2013 at 9:01 AM, sebb <seb...@gmail.com> wrote:
> > >> > AFAIK "accidental" NPEs don't have a message associated with them.
> > >> >
> > >> > So provided that the assertion generates the NPE with a suitable
> > >> > message (e.g.as currently done for the IAE) then it *should* be
> > >> > possible for the end user to distinguish the two cases.
> > >> >
> > >> > I am slightly in favour of retaining IAE as the cause is obvious
> with
> > >> > needing to look at the NPE message.
> > >> >
> > >> > ==
> > >> >
> > >> > Horrible hack: - if you want to use NPE, you could wrap an IAE in
> the
> > >> NPE:
> > >> >
> > >> > npe = new NPE(msg);
> > >> > npe.initCause(new IAE(msg));
> > >> > throw npe;
> > >> >
> > >> > Or vice-vera, of course!
> > >> >
> > >> > Not sure that gains anything, but it does make the stack trace look
> > >> > more impressive!
> > >> >
> > >> > On 30 August 2013 13:42, James Carman <ja...@carmanconsulting.com>
> > >> wrote:
> > >> >> Commons Lang's Validate.notNull() throws NPEs. I don't know that
> > I've
> > >> >> necessarily agreed with that, but at some point a decision was made
> > >> >> that null constraint violations should throw NPEs. Food for
> thought.
> > >> >>
> > >> >> On Fri, Aug 30, 2013 at 8:32 AM, Gary Gregory <
> > garydgreg...@gmail.com>
> > >> wrote:
> > >> >>> On Fri, Aug 30, 2013 at 8:25 AM, Emmanuel Bourg <
> ebo...@apache.org>
> > >> wrote:
> > >> >>>
> > >> >>>>
> > >> >>>> >> + if (parameter == null) {
> > >> >>>> >> + throw new IllegalArgumentException("Parameter '"
> +
> > >> >>>> parameterName + "' must not be null!");
> > >> >>>> >> + }
> > >> >>>> >> + }
> > >> >>>> >> +}
> > >> >>>>
> > >> >>>> Isn't a null value supposed to throw a NPE ?
> > >> >>>>
> > >> >>>
> > >> >>> Not always IMO. When I see an NPE I assume something is very wrong
> > and
> > >> that
> > >> >>> it could be a bug in the impl OR a call site, somewhere on the
> code
> > >> path.
> > >> >>>
> > >> >>> With an IAE, I know for sure it's a problem in the call site
> (which
> > >> could
> > >> >>> be a bug of course).
> > >> >>>
> > >> >>> I does not help that the JRE/JDK is inconsistent, so it's hard to
> > find
> > >> >>> examples.
> > >> >>>
> > >> >>> Gary
> > >> >>>
> > >> >>>
> > >> >>>> Emmanuel Bourg
> > >> >>>>
> > >> >>>>
> > >> >>>>
> > ---------------------------------------------------------------------
> > >> >>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> > >> >>>> For additional commands, e-mail: dev-h...@commons.apache.org
> > >> >>>>
> > >> >>>>
> > >> >>>
> > >> >>>
> > >> >>> --
> > >> >>> E-Mail: garydgreg...@gmail.com | ggreg...@apache.org
> > >> >>> Java Persistence with Hibernate, Second Edition<
> > >> http://www.manning.com/bauer3/>
> > >> >>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/
> >
> > >> >>> Spring Batch in Action <http://www.manning.com/templier/>
> > >> >>> Blog: http://garygregory.wordpress.com
> > >> >>> Home: http://garygregory.com/
> > >> >>> Tweet! http://twitter.com/GaryGregory
> > >> >>
> > >> >>
> ---------------------------------------------------------------------
> > >> >> 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
> > >>
> > >>
> > >
> > >
> > > --
> > > http://people.apache.org/~britter/
> > > http://www.systemoutprintln.de/
> > > http://twitter.com/BenediktRitter
> > > http://github.com/britter
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> > For additional commands, e-mail: dev-h...@commons.apache.org
> >
> >
>
>
> --
> http://people.apache.org/~britter/
> http://www.systemoutprintln.de/
> http://twitter.com/BenediktRitter
> http://github.com/britter
>
