Code Review Request: 7160242: (prefs) Preferences.remove(null) does not throw NPE [macosx]

[Rémi Forax]

On 04/25/2012 12:20 AM, Kurchi Hazra wrote:
> Thanks Remi. I changed it: 
> http://cr.openjdk.java.net/~khazra/7160242/webrev.02/

Thumb up.

>
> Can you also point out what advantage using Object.requireNonNull has 
> over simply doing a key == null check as I was doing before?

Josh Bloch words are better than mine:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2009-October/002891.html

>
> Thanks,
> Kurchi

cheers,
Rémi

>
> On 4/24/2012 3:07 PM, Rémi Forax wrote:
>> On 04/24/2012 11:49 PM, Kurchi Hazra wrote:
>>> Hi,
>>>
>>> Updated webrev:
>>> http://cr.openjdk.java.net/~khazra/7160242/webrev.01/
>>>
>>> Thanks,
>>> Kurchi
>>
>> Hi Kurchi,
>> Object.requireNonNull() return the first argument,
>> so there is no need to store it again in key.
>> So instead of
>>
>> key = Objects.requireNonNull(key, "Specified key cannot be null");
>>
>> you can just write:
>>
>> Objects.requireNonNull(key, "Specified key cannot be null");
>>
>> or if you want to reuse the return value (it's less readable in my 
>> opinion)
>> you can write :
>>
>> file.removeKeyFromNode(path,
>>    Objects.requireNonNull(key, "Specified key cannot be null"));
>>
>> but usually, this feature is used in constructor,
>> something like this :
>>
>> class Person {
>>   ...
>>   public Person(String name) {
>>     this.name = Objects.requireNonNull(name);
>>   }
>> }
>>
>> cheers,
>> Rémi
>>
>>>
>>> On 4/21/2012 4:23 AM, Rémi Forax wrote:
>>>> On 04/21/2012 09:52 AM, Alan Bateman wrote:
>>>>> On 20/04/2012 20:09, Kurchi Subhra Hazra wrote:
>>>>>> Hi,
>>>>>>
>>>>>> This change inserts a null check for the key being passed to 
>>>>>> Preferences.remove() on Mac, so that
>>>>>> the method throws a NullPointerException when key is null 
>>>>>> (according to its specification).
>>>>>>
>>>>>> Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7160242
>>>>>> Webrev: http://cr.openjdk.java.net/~khazra/7160242/webrev.00/ 
>>>>>> Thanks, Kurchi
>>>>> Kurchi - would you be able to add a test to test/java/util/prefs 
>>>>> so that we have coverage for this case?
>>>>>
>>>>> -Alan.
>>>>
>>>> Also you can use Objects.requireNonNull()
>>>> http://docs.oracle.com/javase/7/docs/api/java/util/Objects.html#requireNonNull%28T,%20java.lang.String%29 
>>>>
>>>>
>>>> Rémi
>>>>
>>>
>>
>