Iterator null collection

Question

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!

@LukasEder What default Iterable instance does nullSafe() return? — 卢声远 Shengyuan Lu, Jul 07 '12 at 08:42
I overloaded `nullSafe(List)`, `nullSafe(Set)`, etc. in a utility class... But your idea works as well, as you only expose the `Iterable` type. Concrete `Iterable` implementations shouldn't matter to the call-site — Lukas Eder, Jul 07 '12 at 08:46
And if somebody choose to do code defensive by ignoring nulls, Here is nice way http://stackoverflow.com/a/2250056/1506477 — Thamme Gowda, Apr 30 '14 at 08:17

GETah · Accepted Answer · 2012-07-07T08:55:56.263

6

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!");
   //...
}

edited Jul 07 '12 at 08:55

answered Jul 07 '12 at 08:41

GETah

20,922
7
61
103

Agreed. So I should tell the client code how to use the method and notice the pitfall. I may wrote some Javadoc for the method. – 卢声远 Shengyuan Lu Jul 07 '12 at 08:45
Or throw an `IllegalArgumentException` and return so that client code will know that you do not support `null` collections – GETah Jul 07 '12 at 08:49
*some* may argue like that, yes. But *some* API's are designed to both "support" `null` and empty lists. Take `File.list()` for instance. It can return `null` on some occasions, which aren't necessarily errors. N.B: Also for *some* API's, it may be convenient to accept `null` as a substitute for empty collections, if the semantics would be the same – Lukas Eder Jul 07 '12 at 08:49
@LukasEder I totally agree with you. It all depend on the API you are exposing. – GETah Jul 07 '12 at 08:51
Why do you claim it is a bug? I have such design and I think that collection is not initialized for performance. Why is it a bug? – Val Sep 09 '13 at 07:51

score 0 · Answer 2 · answered Jul 07 '12 at 09:10

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.

score 0 · Answer 3 · answered Jul 07 '12 at 11:04

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
}

Iterator null collection

3 Answers3