commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gary Gregory <garydgreg...@gmail.com>
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 Fri, 30 Aug 2013 18:42:33 GMT
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

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