I would encourage null checking to be done first, rather than sometimes getting StringIndexOutOfBoundsException for a null input. Reasoning about the JDK methods is much easier that way around.
Stephen On 9 January 2013 23:09, Mike Duigou <mike.dui...@oracle.com> wrote: > AbstractStringBuilder probably needs the "Unless otherwise noted," blanket > statement as well (Same as StringBuffer/StringBuilder) > > You might want to move the Objects.requireNonNull(dst); in String.getBytes() > to after the existing checks so as not to unnecessarily change the exception > thrown for bad inputs. An exception will still be thrown but some bad code > may anticipate StringIndexOutOfBoundsException rather than NPE. This is a > very minor matter though. > > Otherwise, it looks good. > > Mike > > On Jan 9 2013, at 14:46 , Jim Gish wrote: > >> Please review the following: >> >> Webrev: http://cr.openjdk.java.net/~jgish/Bug4247235-Add-Blanket-Null-Stmt/ >> <http://cr.openjdk.java.net/%7Ejgish/Bug4247235-Add-Blanket-Null-Stmt/> >> Here's a specdiff: >> http://cr.openjdk.java.net/~jgish/Bug4247235-string-specdiff/ >> <http://cr.openjdk.java.net/%7Ejgish/Bug4247235-string-specdiff/> >> >> >> Summary: This change replaces java/lang/*String*.java javadoc, >> method-specific @throws NullPointerException clauses with blanket >> null-handling statements like that currently in String.java >> >> That was motivated by a discussion awhile back, strongly favoring a blanket >> statement over individual @throws clauses. >> >> For String, the following blanket statement is already in place: >> >> * <p> Unless otherwise noted, passing a <tt>null</tt> argument to a >> constructor >> * or method in this class will cause a {@link NullPointerException} to be >> * thrown. >> >> >> However, despite the blanket statement, the following methods also have >> @throws clauses: >> >> public boolean contains(java.lang.CharSequence s) >> >> Throws: >> |java.lang.NullPointerException|- if|s|is|null| >> --------------------------------------------------------------- >> >> public staticString >> <http://cr.openjdk.java.net/%7Ejgish/Bug4247235-string-specdiff/java/lang/String.html> >> format(String >> <http://cr.openjdk.java.net/%7Ejgish/Bug4247235-string-specdiff/java/lang/String.html> >> format, >> java.lang.Object... args) >> >> >> Throws: >> |java.lang.NullPointerException|- If the|format|is|null >> |----------------------------------------------------------------------- >> || >> >> public staticString >> <http://cr.openjdk.java.net/%7Ejgish/Bug4247235-string-specdiff/java/lang/String.html> >> format(java.util.Locale l, >> String >> <http://cr.openjdk.java.net/%7Ejgish/Bug4247235-string-specdiff/java/lang/String.html> >> format, >> java.lang.Object... args) >> >> Throws: >> |java.lang.NullPointerException|- If the|format|is|null >> |-------------------------------------------------------------------------- >> All of the above are properly specified with the blanket statement or other >> parts of their spec (such as format w.r.t. null Locale) and the @throws can >> safely be removed. >> >> Because the blanket statement is already in effect for String.java, I wrote >> tests for all methods/constructors to ensure compliance. Running them >> revealed the following: >> >> public void getBytes(int srcBegin, int srcEnd, byte dst[], int dstBegin) >> - I would expect an NPE to be thrown if dst == null. However, this isn't >> always the case. If dst isn't accessed because of the values of the other >> parameters (as in getBytes(1,2,(byte[]null,1), then no NPE is thrown. >> - This brings up the question as to the correctness of the blanket statement >> w.r.t. this method. I think this method (and others like it) should use >> Objects.requireNonNull(dst). (The correspoding method public void >> getChars(int srcBegin, int srcEnd, char dst[], int dstBegin), does always >> throw NPE for dst==null) >> >> All other methods/constructors in StringBuffer and StringBuilder either >> correctly state null-handling behavior that differs from the blanket >> statement or are correct w.r.t. the blanket statement. >> >> This has been reviewed by the JCK team. It will require CCC approval >> (because of the new blanket statement, and the fact that some of the >> existing methods were previously silent on null handling, but all already >> conform to the blanket statement). >> >> Thanks, >> Jim Gish >> >> >> -- >> Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304 >> Oracle Java Platform Group | Core Libraries Team >> 35 Network Drive >> Burlington, MA 01803 >> jim.g...@oracle.com >> >> -- >> Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304 >> Oracle Java Platform Group | Core Libraries Team >> 35 Network Drive >> Burlington, MA 01803 >> jim.g...@oracle.com >> >
