commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From James Carman <ja...@carmanconsulting.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 Sun, 01 Sep 2013 18:29:53 GMT
I favor IAE, but I want to make sure we re-visit the decision made for
[lang] when it chose to use NPE instead.

On Sun, Sep 1, 2013 at 1:37 PM, Gary Gregory <garydgregory@gmail.com> wrote:
> It feels to me that this misrepresents the ideas expressed here, or at
> least mine ;) I am neither indifferent or favor NPE. I favor IAE, so
> does that mean I am nor "most people". If a person make these kinds of
> statement, we might as well VOTE or argue-discuss some more...!
>
> Gary
>
> On Sep 1, 2013, at 10:46, Benedikt Ritter <britter@apache.org> wrote:
>
>> 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
>
> ---------------------------------------------------------------------
> 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


Mime
View raw message