Add a Preconditions.chechNoNulls() method for collections

[GoogleCodeExporter]

I'd like to verify a collection contains all non-null elements, which currently 
seems to be most easily done with:

for(Object o : c) {
  checkNotNull(o);
}

Obviously not terrible, but in particular it loses the one-line assignment 
benefit of checkNotNull().  I'd like to be able to do something like:

List<String> safeList = checkNoNulls(unsafeList);

Original issue reported on code.google.com by dimo414 on 15 Aug 2014 at 4:19

[GoogleCodeExporter]

As it turns out, we have this method internally, but we decided to get rid of 
it. The justification:

"""
Advice:
* if you're going to NPE immediately anyway, just do nothing.
* if the NPE might only happen *later*, you should probably be making a 
defensive copy; use Immutable*
* but if you're sure this is what you want, use 
checkArgument(!foos.contains(null)) or checkArgument(!Iterables.contains(foos, 
null))
"""

Original comment by cpov...@google.com on 15 Aug 2014 at 5:59

• Changed state: WorkingAsIntended
• Added labels: Package-Base, Type-Addition

[GoogleCodeExporter]

Basically, asserting facts about the contents of an untrusted *mutable* 
collection is unsatisfying. Who knows if that condition once verified will 
actually remain true?  Defensive copies are a good thing, and 
ImmutableBlah.copyOf() will already do null-checks for you.

Original comment by kevinb@google.com on 15 Aug 2014 at 6:01

[GoogleCodeExporter]

Fair points, and thanks for the checkArgument suggestion (though that raises an 
IAE rather than an NPE, I suppose it's a valid response).  Perhaps my use case 
will explain why I think this would be useful.

I'm implementing a series of forwarding collections (Map, Set, Multimap, 
Multiset) that have some additional behavior, one of which is forbidding null 
keys and values, in the spirit of Guava's immutable collections.  However, 
since the backing collections allow nulls, I have to manually check for nulls 
before I can safely insert into them.

So for instance, I have to do:

@Override
public boolean addAll(Collection<E> c)
{
  for(E e : c) {
    checkNotNull(e);
  }
  return delegate().addAll(c);
}

Rather than simply:

public boolean addAll(Collection<E> c)
{
  return delegate().addAll(checkNoNulls(c));
}

Granted, it's not that much worse, and arguably this is an edge-case need, but 
to me it seemed like a logical Precondition candidate.

Original comment by dimo414 on 15 Aug 2014 at 6:51

[GoogleCodeExporter]

Observation: In Java 8, you could just do 
c.forEach(Preconditions::checkNotNull);

Original comment by lowas...@google.com on 15 Aug 2014 at 6:53

[GoogleCodeExporter]

Note that your code isn't robust when c can be modified by other threads.  One 
of the reasons we recommend Immutable*.copyOf() is that we had to go to great 
pains to get that level of robustness in Guava.

Original comment by kevinb@google.com on 15 Aug 2014 at 6:55

[GoogleCodeExporter]

This issue has been migrated to GitHub.

It can be found at https://github.com/google/guava/issues/<issue id>

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:08

• Added labels: MigratedToGitHub

[GoogleCodeExporter]

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:07

• Added labels: Restrict-AddIssueComment-Commit