specification for null handling in String, StringBuilder, etc.

[Rémi Forax]

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.
>
> David

And I see no problem to check NPE with a str.length if there is a comment
saying something like 'implicit NPE check'.

Rémi

> -----
>
>
>> 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
>>
>>