Skip to content
This repository has been archived by the owner on Jan 22, 2019. It is now read-only.

Guava collection types do not allow null values #52

Merged
merged 1 commit into from
Sep 14, 2015

Conversation

josethomas
Copy link
Contributor

ImmutableMap (and other types) do not allow null values. Trying to store nulls will result in a java.lang.NullPointerException. Can we simply eliminate storing nulls? If this PR acceptable, I can make similar changes for other types as well.

@cowtowncoder
Copy link
Member

Hmmh. This is worth asking on dev list. I agree that getting an NPE is not optimal; but on the other hand, quietly changing the contents can also lead to subtle problems... So perhaps there should be choice, perhaps a module feature to choose between explicit exception that explains the problem, and elimination of nulls?

@cowtowncoder
Copy link
Member

FWIW, I think that

  1. Yes, quietly avoiding nulls for collections that can not store them is the right default behavior, and
  2. It would be great to have a feature to alternatively allow an exception other than NPE to be thrown, instead of quiet swallowing

@ankitcha
Copy link

+1 to both points

@cowtowncoder
Copy link
Member

@josethomas I can merge this, makes sense.

One practical thing we need before merge is to get a filled Contributor Licenses Agreement (CLA), see contributor-agreement.pdf

The usual way is to print it, fill and sign, scan and email to info at fasterxml dot com.
If you could do this, I can merge the patch for inclusion in 2.6.2.
We only need CLA for the first contribution, after which we can accept contributions for all Jackson projects.

@josethomas
Copy link
Contributor Author

CLA sent.

@cowtowncoder cowtowncoder added this to the 2.3. milestone Sep 14, 2015
cowtowncoder added a commit that referenced this pull request Sep 14, 2015
Guava collection types do not allow null values
@cowtowncoder cowtowncoder merged commit e46c441 into FasterXML:master Sep 14, 2015
cowtowncoder added a commit that referenced this pull request Sep 14, 2015
cowtowncoder added a commit that referenced this pull request Sep 14, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants