Elegant null-empty collection checks

[Henryk Konsek]

Hi

What would you think about following Collection helper? It checks
whether given collection is null. If so, it returns new empty
collection (or the original one otherwise).

public static <T> Collection<T> nullToEmpty(Collection<T> collection)
{
if (collection == null) {
return Lists.newLinkedList();
}
return collection;
}

I find it useful when dealing with external APIs which return both
empty and null collections to indicate "no results" return value.

Below I paste example of usage:

import com.google.common.collect.Lists;
import java.util.Collection;
import java.util.List;

public class NullCollectionToEmpty {

public static <T> Collection<T> nullToEmpty(Collection<T>
collection) {
if (collection == null) {
return Lists.newLinkedList();
}
return collection;
}

private static List returnNoResults() {
// This method can return both null and empty collection to
// indicate the lack of results.
return null;
}

public static void main(String[] args) {
// Use case 1 - spaghetti code null-check needed
List nullVulnerableResults = returnNoResults();
if (nullVulnerableResults == null ||
nullVulnerableResults.size() < 2) {
System.out.println("Too few elements.");
}

// Use case 2 - nullToEmpty used / No explicit null-check
needed
List nullSafeResults = returnNoResults();
if (nullToEmpty(nullSafeResults).size() < 2) {
System.out.println("Too few elements.");
}
}

}

[Nikolas Everett]

I think it is a bit much to assume that the user wants an ArrayList when the signature is collection.  If you just plan on using the method to check the results of inconsistent APIs might I suggest an isNullOrEmpty method like commons lang provides for strings?

Nik

> --
> guava-...@googlegroups.com.
> http://groups.google.com/group/guava-discuss?hl=en
> unsubscribe: guava-discus...@googlegroups.com
>
> This list is for discussion; for help, post to Stack Overflow instead:
> http://stackoverflow.com/questions/ask
> Use the tag "guava".

[Henryk Konsek]

I think it is not since empty List is returned just for the purpose of
the postcondition checking. You will use this method only when you
want to perform some kind of postcondition check:

Collection results = ...;
if(emptyToNull(results).size() > 10) {} // check postcondition

If non-empty collection is returned, it is not wrapped in to List.

Regarding isNullOrEmpty - this allows to me only to check if results
are empty or null. I cannot check other postconditions in the same
line (as size() for example).

On 8 Lis, 13:10, Nikolas Everett <nik9...@gmail.com> wrote:
> I think it is a bit much to assume that the user wants an ArrayList when the
> signature is collection.  If you just plan on using the method to check the
> results of inconsistent APIs might I suggest an isNullOrEmpty method like
> commons lang provides for strings?
>
> Nik

> > unsubscribe: guava-discus...@googlegroups.com<guava-discuss%2Bunsubscribe@goog legroups.com>

[Nathan Bryant]

My own codebase has this method:

public static <T> T nvl(T value, T replacement)

Yes, I stole it ;)

[Colin Decker]

One problem with this sort of method is that it would mainly exist just to paper over bad APIs that may return a null Collection. Its existence could also lead to people thinking it's ok to return null Collections since they have this method to convert the result. Other things I don't like:

- it creates a new object when the argument is null

- it's inflexible since it can only return Collection... what if you need it to have List type for your postcondition check, or the original Collection was a Set and you want to later return the result as a Set?

Guava already has a far more flexible method that could be used here: Objects.firstNonNull(T, T). That plus the the Collections.empty* collections can do what you want:

    if (firstNonNull(results, emptyList()).size() > 10) { ... }

This has the advantage of not creating a new object if results is null as well as being able to return List, Set, SortedSet, Multiset or whatever. It's not quite as compact as your method, but I think that's fine because it shouldn't be something you have to do too often and I feel like the flexibility more than makes up for the additional verbosity.

Colin

[Kevin Bourrillion]

And of course the straightforward option (if you can stomach reassigning a local) is

    Set<Foo> foos = NaughtyClass.getFoos();

    if (foos == null) {

      foos = ImmutableSet.of();

    }

If that's less than perfectly elegant, the fault lies squarely with NaughtyClass, which has no good reason to act this way. 

And if this isn't a null being returned to you, but rather passed to you, just let an NPE throw.

--
Kevin Bourrillion @ Google
http://guava-libraries.googlecode.com

[lacroix1547]

This is dummy proofing.
I like dummy proofing.
Removing all dummyness from the planet would be fine too.

[Henryk Konsek]

I'll create two methods then. One for Sets and and one for Lists. It
will be a compromise between my brief notation and List/Set
distinction.

Thanks for ideas.