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
