CODEWITHSUNDEEP
javacollectionsiterator
卢声远 Shengyuan Lu
卢声远 Shengyuan Lu

Reputation: 32004

Iterator null collection

It's quite common that I have to check null before iterate when not sure the collection reference is null or not. Sample:

Collection<Object> collection = ...
...
if(collection != null)//troublesome
    for(Object o : collection)

Of course, I know empty collection is much better than null, but in some cases client code cannot control the nullable collection from other modules (for instance, return value from 3rd party code). So I wrote a utility method:

public static <T> Iterable<T> nullableIterable(Iterable<T> it){
    return it != null ? it : Collections.<T>emptySet();
}

In client code, no need to check null any more:

for(Object o : nullableIterable(collection))
...

Do you think nullableIterable() is reasonable? Any advice? Any concern? Thanks!

Upvotes: 10

Views: 11176

Answers (3)

Yair Zaslavsky
Yair Zaslavsky

Reputation: 4137

In most cases this would be OK.
Bare in mind you might encounter third parties that return null in case of error , and that an empty list is a valid result.
I would therefore consider to alter a bit your code and do something like this:

public static <T> Iterable<T> nullableIterable(Iterable<T> it, boolean exceptionIfNull){
    if (exceptionIfNull && it == null) {
        throw new NUllPointerException("Iterable is null");
    } else
    return it != null ? it : Collections.<T>emptySet();
}

public static <T> Iterable<T> nullableIterable(Iterable<T> it){
    return nul,lableIterable(it,false); //Default behavior for most cases
}

Upvotes: 0

Volodymyr Shtenovych
Volodymyr Shtenovych

Reputation: 162

This looks good to me if you limit usage of this function to the layer that interacts with "external" code and make sure you will never start using it for defense from yourself or from your colleagues. Consider annotate parameters and fields within your code with @Nullable annotation - assuming that what is not annotated cannot be null, pretty helpful especially taking into account that IDEs and static analysis tools are aware of this annotation.

Upvotes: 0

GETah
GETah

Reputation: 21419

That looks good. I personally do that too. You will always get developers who would disagree with this as it is kind of defensive programming. Imagine you have a workflow or a class that is not supposed to return null. This means that getting a null from it is a bug which your code will hide as it will turn the null to an empty collection and the bug will never surface.

If you are for example writing APIs that do not support null collections then you should avoid this. If client code gives you a null collection where you do not support it, you should throw an IllegalArgumentException to let client code know that there is something wrong with the provided collection. Something like:

public void myApiNoSupportForNull(Collection<Object> collection){
   // Pre condition
   if(collection == null) 
     throw new IllegalArgumentException("This API does not support null collections!");
   //...
}

Upvotes: 6

Related Questions