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

Rémi Forax forax at univ-mlv.fr
Tue Apr 24 22:44:58 UTC 2012

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/viewbug.do?bugid=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

More information about the core-libs-dev mailing list