Return-Path: X-Original-To: apmail-commons-dev-archive@www.apache.org Delivered-To: apmail-commons-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 769F6104BA for ; Fri, 30 Aug 2013 16:14:18 +0000 (UTC) Received: (qmail 59461 invoked by uid 500); 30 Aug 2013 16:14:17 -0000 Delivered-To: apmail-commons-dev-archive@commons.apache.org Received: (qmail 59216 invoked by uid 500); 30 Aug 2013 16:14:17 -0000 Mailing-List: contact dev-help@commons.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: "Commons Developers List" Delivered-To: mailing list dev@commons.apache.org Received: (qmail 59206 invoked by uid 99); 30 Aug 2013 16:14:16 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 30 Aug 2013 16:14:16 +0000 X-ASF-Spam-Status: No, hits=0.3 required=5.0 tests=FREEMAIL_REPLY,RCVD_IN_DNSWL_LOW,SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (nike.apache.org: domain of sebbaz@gmail.com designates 74.125.82.174 as permitted sender) Received: from [74.125.82.174] (HELO mail-we0-f174.google.com) (74.125.82.174) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 30 Aug 2013 16:14:11 +0000 Received: by mail-we0-f174.google.com with SMTP id q54so1769311wes.19 for ; Fri, 30 Aug 2013 09:13:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :content-type; bh=IuVKkVsFHHMlKM6J5WPnorSnC14pvI6fxeKTY+Q8cxs=; b=hc6BsJE/65E7+nL35DLak4MU2gWkbWVDjOnZrFMeOxtvORmBOJldKJAuEIWYw+xFDq 00Z6UnEpUxqv/gD/bujJafeKMOzqgYMWi3IJidn8RUxkzYHK9SIiTBo8ZHPsfoc+Zkus 9lgGaorXHiSB4QHcy7vG/wuE+g1mOjosQBK99Eu1kqoIXXXGOnG4FqPBz9gnANYKY52p yv9F2Pmvs/eeMj39c5Th0DsnXw5Nf6ImmooXljkK3+MVKqOeRmcc1cfyFvjoq+Vn//4X Vn2eg4ABt73+MH9Up7sAF1yBafGe/T5c6wlVZq4EcEn3Y8QlhRR7yX3jYncLbgrtnKl2 is3g== MIME-Version: 1.0 X-Received: by 10.180.208.7 with SMTP id ma7mr663049wic.25.1377879230889; Fri, 30 Aug 2013 09:13:50 -0700 (PDT) Received: by 10.194.16.167 with HTTP; Fri, 30 Aug 2013 09:13:50 -0700 (PDT) In-Reply-To: <5220BF50.50206@sandglass-software.com> References: <20130829201814.3B9672388900@eris.apache.org> <52208F25.5000708@apache.org>

<5220BF50.50206@sandglass-software.com> Date: Fri, 30 Aug 2013 17:13:50 +0100 Message-ID: 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/ From: sebb To: Commons Developers List Content-Type: text/plain; charset=ISO-8859-1 X-Virus-Checked: Checked by ClamAV on apache.org The JDK Javadoc says of NPE: * Applications should throw instances of this class to indicate * other illegal uses of the null 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 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 >> wrote: >> >>> 2013/8/30 sebb >>> >>>> On 30 August 2013 15:19, Benedikt Ritter 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 >>>>> >>>>>> 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 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 >>>>>> >>>>>> 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 >>>>>>>> Spring Batch in Action >>>>>>>>> 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