views:

541

answers:

4

What is the best way to guard against null in a for loop in Java?

This seems ugly :

if (someList != null) {
    for (Object object : someList) {
        // do whatever
    }
}

Or

if (someList == null) {
    return; // Or throw ex
}
for (Object object : someList) {
    // do whatever
}

There might not be any other way. Should they have put it in the for construct itself, if it is null then don't run the loop?

+6  A: 

You could potentially write a helper method which returned an empty sequence if you passed in null:

public static <T> Iterable<T> emptyIfNull(Iterable<T> iterable) {
    return iterable == null ? Collections.<T>emptyList() : iterable;
}

Then use:

for (Object object : emptyIfNull(someList)) {
}

I don't think I'd actually do that though - I'd usually use your second form. In particular, the "or throw ex" is important - if it really shouldn't be null, you should definitely throw an exception. You know that something has gone wrong, but you don't know the extent of the damage. Abort early.

Jon Skeet
I would change the Iterable<T> list parameter to Iterable<T> iterable, as not every iterable is a list.
Lombo
@limbo: Yes, good call.
Jon Skeet
+6  A: 

You should better verify where you get that list from.

An empty list is all you need, because an empty list won't fail.

If you get this list from somewhere else and don't know if it is ok or not you could create a utility method and use it like this:

for( Object o : safe( list ) ) {
   // do whatever 
 }

And of course safe would be:

public static List safe( List other ) {
    return other == null ? Collections.EMPTY_LIST : other;
}
OscarRyz
Note that Collections.emptyList() will avoid allocating an extra object (IIRC).
Jon Skeet
@Jon: I have always asked my self, what was the use of that `emptyList` http://java.sun.com/j2se/1.5.0/docs/api/java/util/Collections.html#emptyList() What's IIRC?
OscarRyz
IIRC = "If I recall correctly". And yes, there is a singleton instance that is returned for all calls to Collections.emptyList().
ColinD
Collections.Emptylist is iirc a singleton already.
Thorbjørn Ravn Andersen
A: 

If you are getting that List from a method call that you implement, then don't return null, return an empty List.

If you can't change the implementation then you are stuck with the null check. If it should't be null, then throw an exception.

I would not go for the helper method that returns an empty list because it may be useful some times but then you would get used to call it in every loop you make possibly hiding some bugs.

Lombo
A: 

There's the tried-and-true C-style defense:

for (int i = 0; someList!= null && i < someList.size(); i++) {
    Object o = someList.get(i);
    // ...
}

get() is less efficient than iterating through the list for some collection implementations, but the meaning here is clear and concise.

mobrule
This not a great alternative to do as a matter of course. It introduces an additional test on every iteration of the loop (which *might* be optimized out at runtime, but is still more to assimilate for the next guy looking at the code), and it uses the potentially horribly inefficient get(i) unnecessarily. It's also needlessly more verbose.
Software Monkey