specification for null handling in String, StringBuilder, etc.
forax at univ-mlv.fr
Tue Jun 12 06:54:12 UTC 2012
On 06/12/2012 07:40 AM, David Holmes wrote:
> 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.
And I see no problem to check NPE with a str.length if there is a comment
saying something like 'implicit NPE check'.
>> 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 ||
>> 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.
>> Jim Gish
More information about the core-libs-dev