Code Review Request: 7165118: (prefs) AbstractPreferences.remove(null) does not throw NPE

[Chris Hegarty]

On 02/05/2012 10:55, David Holmes wrote:
> On 2/05/2012 7:53 PM, Chris Hegarty wrote:
>> On 02/05/2012 04:54, David Holmes wrote:
>>> Hi Kurchi,
>>>
>>> You should also add:
>>>
>>> @throws NullPointerException {@inheritDoc}
>>
>> Right, it would be best to clarify this in the AbstractPreferences
>> specification, similar to the other putXXX, getXXX methods.
>>
>>> to the method spec so that the docs re-instate the fact that it is
>>> supposed to throw NPE. As it stands I could argue that
>>> AbstractPreferences.remove has chosen not to throw NPE for a null key -
>>> leaving it up to removeSpi to do that if needed.
>>>
>>> CCC may be needed for this.
>>
>> Yes, we should track this minor spec clarification (to document existing
>> behavior). If we have agreement on list, then we can take this offline
>> and complete the paper work ;-)
>
> It's not actually documenting existing behaviour as currently we don't
> throw the NPE. The assumption is that we should have been throwing the
> NPE. So this is a spec and behaviour change.

D'oh, sorry obviously you're right. Somehow I overlooked most important 
line of this change! Anyway, you are right we should update the docs.

-Chris.

>
> David
>
>> Oh, nice to see a test added for this too.
>>
>> -Chris.
>>
>>>
>>> David
>>>
>>>
>>> On 2/05/2012 5:01 AM, Kurchi Hazra wrote:
>>>> Hi,
>>>>
>>>> This is a simple fix to enable AbstractPreferences.remove() to check
>>>> for
>>>> a null argument and
>>>> throw a NullPointerException if required.
>>>> I have also modified test/java/util/prefs/RemoveNullKeyCheck.java to
>>>> cover this case.
>>>>
>>>> Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7165118
>>>> Webrev: http://cr.openjdk.java.net/~khazra/7165118/webrev.00/
>>>>
>>>> Thanks,
>>>> Kurchi