commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@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 16:13:50 GMT
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


Mime
View raw message