commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benedikt Ritter <brit...@apache.org>
Subject Re: [spam] Re: svn commit: r1518802 - in /commons/proper/csv/trunk/src: main/java/org/apache/commons/csv/ test/java/org/apache/commons/csv/
Date Sun, 01 Sep 2013 14:46:01 GMT
Look's like most people are either indifferent or favor NPE. So, do we
change this for [CSV]? The important thing is to give users an expressive
message.

Benedikt



2013/8/30 Gary Gregory <garydgregory@gmail.com>

> Surprisingly, a lot. At work, we have a lot of frameworky/plugin-type of
> code where we run operations on collections of things and we do not want
> "expected" errors to torpedo the whole processing flow, so we do catch
> things like IAE and ISE. We do try to avoid catching Exception if we can
> help it.
>
> Gary
>
>
> On Fri, Aug 30, 2013 at 2:32 PM, Matt Benson <gudnabrsam@gmail.com> wrote:
>
> > How often do you really want to catch these?
> >
> > Matt
> >
> >
> > On Fri, Aug 30, 2013 at 1:20 PM, Gary Gregory <garydgregory@gmail.com
> > >wrote:
> >
> > > Another perspective to think about is whether you want to write code
> > like:
> > >
> > > try {
> > >   // la-di-da
> > > } catch (NullPointerException e) {
> > >   // garbage in!
> > > }
> > >
> > > or:
> > >
> > > try {
> > >   // doo-wap-doo-wap
> > > } catch (IllegalArugumentException e) {
> > >   // garbage in!
> > > }
> > >
> > > Catching NPE just smells funny to me.
> > >
> > > Gary
> > >
> > >
> > >
> > > On Fri, Aug 30, 2013 at 1:06 PM, sebb <sebbaz@gmail.com> wrote:
> > >
> > > > The fact that NPE is documented in Bloch is quite important.
> > > >
> > > > Whatever we do choose, we should make sure to document all th reasons
> > > > (pros and cons) somewhere other than just the mailing list!
> > > >
> > > > On 30 August 2013 17:30, Matt Benson <gudnabrsam@gmail.com> wrote:
> > > > > The discussion for [lang], none of whose participants have weighed
> in
> > > > here,
> > > > > took place in late 2009 (so perhaps a little longer ago than I
> > thought)
> > > > and
> > > > > is archived at [1].  IMO Paul B. makes some pretty compelling
> > arguments
> > > > in
> > > > > [2].
> > > > >
> > > > > Matt
> > > > >
> > > > > [1] http://markmail.org/thread/7gw7xzrc3c3ul74c
> > > > > [2] http://markmail.org/message/wmc4jh4pmhjjtxdf
> > > > >
> > > > >
> > > > > On Fri, Aug 30, 2013 at 11:13 AM, sebb <sebbaz@gmail.com> wrote:
> > > > >
> > > > >> The JDK Javadoc says of NPE:
> > > > >>
> > > > >>  * Applications should throw instances of this class to indicate
> > > > >>  * other illegal uses of the <code>null</code> object.
> > > > >>
> > > > >> and of IAE:
> > > > >>
> > > > >>  * Thrown to indicate that a method has been passed an illegal
or
> > > > >>  * inappropriate argument.
> > > > >>
> > > > >> That says to me that we should throw IAE here.
> > > > >>
> > > > >> The JDK does use NPE for parameter checks, but it also uses IAE,
> for
> > > > >> example:
> > > > >>
> > > > >> javax.management.monitor.Monitor.addObservedObject
> > > > >> java.rmi.activation.ActivationDesc.ActivationDesc
> > > > >> javax.management.relation.RoleList.add
> > > > >> javax.imageio.metadata.IIOMetadataFormatImpl.addAttribute
> > > > >>
> > > > >> On 30 August 2013 16:50, Adrian Crum <
> > > > adrian.crum@sandglass-software.com>
> > > > >> wrote:
> > > > >> > I've seen a lot of discussions on NPE versus IAE, and in
the end
> > > they
> > > > all
> > > > >> > condense down to what Matt stated here: Those who favor
NPE cite
> > the
> > > > JDK
> > > > >> > classes as a pattern to follow, and those who favor IAE
say it
> is
> > a
> > > > >> better
> > > > >> > description of the problem. From my perspective, both are
valid
> > > > >> viewpoints,
> > > > >> > and a project simply needs to choose one. In the end, the
choice
> > is
> > > > >> neither
> > > > >> > "right" nor "wrong" - it is just a choice.
> > > > >> >
> > > > >> > -Adrian
> > > > >> >
> > > > >> >
> > > > >> >
> > > > >> > On 8/30/2013 8:07 AM, Matt Benson wrote:
> > > > >> >>
> > > > >> >> 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
> > > > >> >>>
> > > > >> >
> > > > >> >
> > > > >> >
> > > ---------------------------------------------------------------------
> > > > >> > 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
> > > >
> > > >
> > >
> > >
> > > --
> > > 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
> > >
> >
>
>
>
> --
> 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
>



-- 
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