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

[Chris Hegarty]

On 24/04/12 23:44, Rémi Forax wrote:
> 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.

+1.

I also really like requireNonNull. To me its less verbose, cleaner, and 
the intent is easily understood.

The jtreg tags/comments look a little funny, but shouldn't cause a 
problem. Typically we something like ( note the extra stars! ).

/* @test
  * @bug 7160242
  * @summary Check if NullPointerException is thrown if the key passed
  *          to remove() is null.
  */

Anyway, looks like you are good to push.

-Chris.

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