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

Chris Hegarty chris.hegarty at oracle.com
Wed May 2 09:57:24 UTC 2012

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

More information about the core-libs-dev mailing list