Cache behavior on null keys/values

[Yannis Cosmadopoulos]

I can't remember for sure if we made any decision on the permissibility of nulls in key/value of Cache entry, though I seem to remember that we said neither should be allowed. That may be just because it is my preference.

Here is what Map says:
http://download.oracle.com/javase/6/docs/api/java/util/Map.html#put%28K,%20V%29

Specific questions, given a Cache<Integer, String>

1) Are null keys allowed? My preference is NO.
    if no,
    1a) cache.put(null, "v1") will throw an IllegalArgumentException (this is my preference)
    1b) cache.put(null, "v1") will throw an NullPointerException (this is what Map proposes)
    1c) cache.put(null, "v1") silently does nothing
2) Are null values allowed? my preference is NO.
    if no,
    2a) cache.put(1, null) will throw an IllegalArgumentException (this is my preference)
    2b) cache.put(1, null) will throw an NullPointerException (this is what Map proposes)
    2c) cache.put(1, null) silently does nothing

Whatever the answers to these will propagate to other methods including putAll, remove, get

--Yannis

--
I will be participating (again) in the ALC bicycle ride, from San Francisco to Los Angeles, raising money to fight AIDS. Please support me: http://www.tofighthiv.org/goto/yannis

[Tim Eck]

+1 on disallowing null keys and values

 

The question of NPE vs IllegalArgumentException isn’t black and white to me. On this exact topic in the past though I’ve delegated to Josh Bloch’s guidance in Effective Java (item 60 in 2^nd ed)

 

You can find the text of that item in this thread on the exact topic:

http://stackoverflow.com/questions/3881/illegalargumentexception-or-nullpointerexception-for-a-null-parameter

[Ben Cotton]

+1 on disallowing null keys and values

+0 on handling the Exception via NPE or IAE (even after reading Tim's
excellent reference, I can't really decide which side to take in this
'holy war' on exception use)

On May 26, 1:53 pm, "Tim Eck" <tim...@gmail.com> wrote:
> +1 on disallowing null keys and values
>
> The question of NPE vs IllegalArgumentException isn't black and white to me.
> On this exact topic in the past though I've delegated to Josh Bloch's
> guidance in Effective Java (item 60 in 2nd ed)
>
> You can find the text of that item in this thread on the exact topic:
>

> http://stackoverflow.com/questions/3881/illegalargumentexception-or-n...
> terexception-for-a-null-parameter

>
> From: jsr...@googlegroups.com [mailto:jsr...@googlegroups.com] On Behalf Of
> Yannis Cosmadopoulos
> Sent: Thursday, May 26, 2011 10:44 AM
> To: jsr...@googlegroups.com
> Subject: Cache behavior on null keys/values
>
> I can't remember for sure if we made any decision on the permissibility of
> nulls in key/value of Cache entry, though I seem to remember that we said
> neither should be allowed. That may be just because it is my preference.
>

> Here is what Map says:http://download.oracle.com/javase/6/docs/api/java/util/Map.html#put%2...
> %29

[Yannis Cosmadopoulos]

I revise my opinion.
Seeing Map thrown NPE and Effective Java recommends it I vote for NPE on null key+value.

[Greg Luck]

The Cache is not a Map. A Map is used in a lot of contexts. A Map has views whereas we just have an Iterator. I am not inclined to reason about Cache behaviour _solely_ on what Map does.

My view on this for years has been that caches should be easy to use, robust and not surprise developers. 

Established precedent here from Ehcache is to ignore null keys and null values. Further get(null) returns null. None through any exception.

Infinispan follows this precedent.

Thinking back through the last 8 years I cannot recall this behaviour causing problems with Ehcache.

So I think we should do 1c and 2c.

On 27/05/2011, at 3:43 AM, Yannis Cosmadopoulos wrote:

I can't remember for sure if we made any decision on the permissibility of nulls in key/value of Cache entry, though I seem to remember that we said neither should be allowed. That may be just because it is my preference.

Here is what Map says:
http://download.oracle.com/javase/6/docs/api/java/util/Map.html#put%28K,%20V%29

Specific questions, given a Cache<Integer, String>

1) Are null keys allowed? My preference is NO.
    if no,
    1a) cache.put(null, "v1") will throw an IllegalArgumentException (this is my preference)
    1b) cache.put(null, "v1") will throw an NullPointerException (this is what Map proposes)
    1c) cache.put(null, "v1") silently does nothing

2) Are null values allowed? my preference is NO.
    if no,
    2a) cache.put(1, null) will throw an IllegalArgumentException (this is my preference)
    2b) cache.put(1, null) will throw an NullPointerException (this is what Map proposes)
    2c) cache.put(1, null) silently does nothing

Whatever the answers to these will propagate to other methods including putAll, remove, get

--Yannis

--
I will be participating (again) in the ALC bicycle ride, from San Francisco to Los Angeles, raising money to fight AIDS. Please support me: http://www.tofighthiv.org/goto/yannis

Regards

Greg Luck

web: http://gregluck.com

skype: gregrluck

yahoo: gregrluck

mobile: +61 408 061 622

[Nikita Ivanov]

Swallowing legitimate NPE/IAP seems strange to me. 

If we are to put a strong contract that no 'nulls' allowed (which I support) why not give the user fail-fast API that throws NPE/IAP (runtime exception) instead of pushing people down the rabbit hole of debugging of "why is my put not working?"...

What is the argument for purposefully hiding the bug in user logic? If anything - I'd additionally have something along the lines of putSafe(K, V) which would ignore nulls if passed.

Thoughts?

--

Nikita Ivanov, CEO

GridGain Systems

www.gridgain.com

[Manik Surtani]

I agree with Nikita - silently swallowing stuff is generally bad API design.

Greg, you are wrong about Infinispan being silent on null keys/values:

https://github.com/infinispan/infinispan/blob/5.0.0.CR4/core/src/main/java/org/infinispan/CacheDelegate.java#L233

So, my vote is NPE (rather than IAE) on a null key or value being passed in to any Cache API method.

- Manik