commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Matt Benson <gudnabr...@gmail.com>
Subject Re: svn commit: r1518802 - in /commons/proper/csv/trunk/src: main/java/org/apache/commons/csv/ test/java/org/apache/commons/csv/
Date Fri, 30 Aug 2013 15:07:50 GMT
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 <britter@apache.org> wrote:

> 2013/8/30 sebb <sebbaz@gmail.com>
>
> > On 30 August 2013 15:19, Benedikt Ritter <britter@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 <james@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 <sebbaz@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 <james@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 <
> > garydgregory@gmail.com>
> > >> wrote:
> > >> >>> On Fri, Aug 30, 2013 at 8:25 AM, Emmanuel Bourg <
> ebourg@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-unsubscribe@commons.apache.org
> > >> >>>> For additional commands, e-mail: dev-help@commons.apache.org
> > >> >>>>
> > >> >>>>
> > >> >>>
> > >> >>>
> > >> >>> --
> > >> >>> E-Mail: garydgregory@gmail.com | ggregory@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-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
> > >>
> > >>
> > >
> > >
> > > --
> > > http://people.apache.org/~britter/
> > > http://www.systemoutprintln.de/
> > > http://twitter.com/BenediktRitter
> > > http://github.com/britter
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
> >
>
>
> --
> http://people.apache.org/~britter/
> http://www.systemoutprintln.de/
> http://twitter.com/BenediktRitter
> http://github.com/britter
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message