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

Kurchi Hazra kurchi.subhra.hazra at oracle.com
Wed May 2 17:47:14 UTC 2012

Thanks all for the review. A webrev with the doc change: http://cr.openjdk.java.net/~khazra/7165118/webrev.01/

I will go ahead and file a CCC for this.

Thanks, Kurchi

On 5/2/2012 2:57 AM, Chris Hegarty wrote:

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/viewbug.do?bugid=7165118 Webrev: http://cr.openjdk.java.net/~khazra/7165118/webrev.00/ Thanks, Kurchi

-- -Kurchi

More information about the core-libs-dev mailing list