Hi Jim,
On 12/06/2012 7:59 AM, Jim Gish wrote:
While triaging outstanding String bugs, I was looking at 4247235, "(spec
str) StringBuffer.insert(int, char[]) specification is inconsistent"
Although the description is out of date w.r.t. the current code, I did
find what I would consider weaknesses (maybe even holes) in the specs
relative to this issue.
It appears that the common practice is to spec checked exceptions but
not unchecked exceptions (which I understand). For example, in the case
mentioned the spec indicates that StringIndexOutOfBoundsException is
thrown if the offset is invalid, and is silent about the consequences of
the char[] being null. In the latter case, it throws
NullPointerException, but the str == null is not checked, rather it
simply tries to access str.length and fails if str is null.
My personal feeling is that pre-conditions should be explicitly checked
for and be spec'd.
This is very, very common in the core libraries. Rather than document
every single method where a null parameter triggers NPE they are often
covered (directly or indirectly) by blanket statements of the form
"unless otherwise stated ...".
I'm strongly of the opinion that people should be reading the complete
specification for any type they use, not just individual methods. So I
don't have a problem with not documenting this on each method - there
are likely to be far more significant misunderstandings of behaviour if
you don't read all the docs. But I understand others will see this from
the other side.
Also note that the handling of unchecked exceptions by JavaDoc
complicates things because overriding implementations need to re-state
that they throw the NPE (or whatever it may be), using @inheritDoc, even
if there is no change from the superclass or interface specification of
the method.
David
-----
One might argue that the spec is complete, because it says that "The
overall effect is exactly as if the second argument were converted to a
string by String.valueOf( char[] ),..." If you look at the class javadoc
for String there is a statement that "Unless otherwise noted, passing a
null argument to a constructor or method in this class will cause a ||
<http://docs.oracle.com/javase/7/docs/api/java/lang/NullPointerException.html>|NullPointerException|
to be thrown." However, if the user simply looks at the javadoc for
String.valueOf( char[] ) nothing is said about null handling there. The
user would have to have read the class javadoc to catch the reference to
NullPointerException. Seems like an unreasonably indirect chain to have
to follow, when all we'd have to say is that the original insert method
throws NPE if the char[] is null.
I suggest we improve the readability here (and in related places) and
tell the user straight off the effect of passing null.
Cheers
Jim Gish
