On 5/6/14, 10:09 AM, sebb wrote: > On 6 May 2014 14:27, Benedikt Ritter <brit...@apache.org> wrote: >> Hi Thiago, >> >> >> 2014-05-06 14:53 GMT+02:00 Thiago Andrade <thia...@gmail.com>: >> >>> Hello people, >>> >>> Analizing the JIRA issue https://issues.apache.org/jira/browse/LANG-1008the >>> contributors noticed that NumberUtils.max/min methods all have the same >>> problem: >>> They all throw an IllegalArgumentException when according to the official >>> documentation (Oracle|Sun) says that a NullPointerException must be thrown >>> when an argument must not be null. >>> >> This is not a problem imho. It is a question of API design. > +1 > >> I don't now an >> offical documentation that say when IAE or NPE _must_ be thrown. Sun/Oracle >> at some point decided to throw NPE when ever a null reference is passed to >> a method that doesn't accept null inputs. I don't feel this is right, since >> a null input is also an illegal argument. Why make a differenciation? IMHO >> NPE should be reserved to the JVM, when a method is called on a null >> reference, but that's just my opinion. >> > +1. > > NPE used to mean a bug had occurred rather than the user had provided bad > input. > > Using NPE for a parameter that must not be null confuses things. > >>> However according to Apache Commons Lang Developer Guide, these methods are >>> all correct. This guide says that "When throwing an exception to indicate a >>> bad argument, always try to throw IllegalArgumentException, even if the >>> argument was null. Do not throw NullPointerException.". >>> >> Since [lang] is currently designed this way, I'd rather deal with this >> issue for 4.0. We can then revisit our initial decision to only throw IAE >> an maybe align it to what the JDK now does. If you want to file an issue, >> my opinion is, that it should be fix version 4.0. Changing the exceptions >> that are thrown now may break clients (although I think there are very few >> use cases where one should catch IAE or NPE). > If Commons ever decide to switch to NPE (I hope not) then it is > imperative that the message is 100% clear that the problem is with a > method argument, and which argument is at fault. > > Otherwise we will likely find ourselves fielding bug reports about > Commons code when it is the caller that is at fault. > Even then, I suspect some reporters will just see the NPE and assume > that the Commons code has a bug. > > If an argument is invalid, throw IAE. > IMO it does not make sense to throw NPE for some invalid arguments and > not others. > What Sun/Oracle perhaps should have done was introduce an > "InvalidNullArgumentException" > > The Javadoc (1.7) says: > > Thrown when an application attempts to use null in a case where an > object is required. These include: > > Calling the instance method of a null object. > Accessing or modifying the field of a null object. > Taking the length of null as if it were an array. > Accessing or modifying the slots of null as if it were an array. > Throwing null as if it were a Throwable value. > > Applications should throw instances of this class to indicate other > illegal uses of the null object. > <<< > > I suppose "illegal use of the null object" could be taken to mean > passing null to a non-nullable parameter, but I think that is > stretching it too far.
+1 to everything above. The "illegal use of the null object" bit is an unfortunate choice of words as it seems to have opened the door to the s/IAE/NPE debate. I am OK with "throw NPE early when you know it is going to happen further down" approach when API doc does not specify behavior, but I prefer APIs that document their preconditions clearly and throw IAE when actual parameters violate those preconditions. Phil > >>> This mail was sent in order to discuss around and make decisions to solve >>> this dilemma where the Java official specification says X and the Apache >>> official specification says Y. >>> >> Can you please provide a lnk to the official specification you're refering >> to? ;-) >> >> Regards, >> Benedikt >> >> >>> My sincere thanks for your time and consideration, >>> >>> -- >>> Thiago Andrade >>> >> >> >> -- >> http://people.apache.org/~britter/ >> http://www.systemoutprintln.de/ >> http://twitter.com/BenediktRitter >> http://github.com/britter > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
