Java null pointer checking for faster code execution

Question

Imagine a method which takes an object as a parameter and check with a for each loop for lets say some other values inside of some collection, which can be found/filtered according to the passed in argument. Is it a good practice to check at the beginning of the function for null pointer and return immediately an empty collection or null pointer, or is it better to just leave out the null pointer checking, as for each loop takes care of it, but the function will need more time to execute (because of the whole for each iteration). And lets say that this collection isn't big (not that much time consuming).

 public ArrayList<Foo> find(Bar bar) {  
        if (bar == null) { // get rid of these part?
            return null;   //
        }                  //  

        ArrayList<Foo> foos = new ArrayList<Foo>();
        for (Foo f: Foo.values()) {
            if (f.someBarCollection.contains(bar)) {
                foos.add(f);
            }
        }

        return foos;
 }

I think It's better to check for null and return immediatelly if you know that It's a waste of time doing any further actions, because you know they're not needed. So I'm favouring semantics at the expense of a shorter code, to make things unambigous.

EDIT: Let me elaborate a bit further. The result of the function is the same with OR without the null checking part. The question is just, should I check It anyway, just for the sake of better expression (and a bit of performance gain, but this is not a problem), but the code will be longer (because of the added checking)?

Answer 1

One thing to consider is future-proofing.

If you can imagine a time when the collection legally contains(null) then either let it run through (and return an empty collection) or throw an exception (probably IllegalArgumentException). Throwing an exception will mean that change won't affect the behavior of existing legal code.

Unless you considered returning null for 'empty' when bar!=null (not normally recommended) it's a little inconsistent to return it for bar==null.

Quite rightly most of the other posts discourage null for 'empty'. The only reason to use it would be performance:

//WARNING - THIS CODE IS NOT FIRST CHOICE
public ArrayList<Foo> find(Bar bar) {  
        ArrayList<Foo> foos = null;
        for (Foo f: Foo.values()) {
            if (f.someBarCollection.contains(bar)) {
                if(foos==null){
                    foos=new ArrayList<Foo>();
                }
                foos.add(f);
            }
        }
        return foos;
 }

Collections.emptyList() is valuable but any calling code that thinks it can modify the list will get a nasty surprise of UnsupportedOperationException. So replacing null with 'empty' can risk exchanging NullPointerException for (possibly a less frequent and harder to spot) UnsupportedOperationException. Not as much progress as we'd like.

The next alternative of returning a local ArrayList is frightening:

//DANGER - CODING HORROR!

static final sEmpty=new ArrayList<Foo>();

public ArrayList<Foo> find(Bar bar) {  
        ArrayList<Foo> foos = null;
        for (Foo f: Foo.values()) {
            if (f.someBarCollection.contains(bar)) {
                if(foos==null){
                    foos=new ArrayList<Foo>();
                }
                foos.add(f);
            }
        }
        return foos==null?sEmpty:foos;
 }

In this case ignorant code might add elements to a supposedly empty ArrayList and cause them to be returned by subsequent calls to find(). A great way to spend a day is finding that one!

Sadly, it seems we're as far from Java implementing C++ style const to differentiate mutable and immutable references than ever. So in at least some cases you're left with returning small junky objects or null for 'empty'.

At the end of the day the Java libraries take the attitude the creating and destroying lots of small objects is an acceptable overhead for simplicity (look at 'boxed' primitives like Integer). Many modern JVMs have generational garbage collectors designed to harmonise with that style.

So if you're undecided about 'accepting' null:

public ArrayList<Foo> find(Bar bar) {  
        if(bar==null){
            throw new IllegalArgumentException("bar==null");
        }
        ArrayList<Foo> foos = new ArrayList<Foo>();
        for (Foo f: Foo.values()) {
            if (f.someBarCollection.contains(bar)) {
                foos.add(f);
            }
        }
        return foos;
 }

Otherwise go with the flow:

public ArrayList<Foo> find(Bar bar) {  
        ArrayList<Foo> foos = new ArrayList<Foo>();
        for (Foo f: Foo.values()) {
            if (f.someBarCollection.contains(bar)) {
                foos.add(f);
            }
        }
        return foos;
 }

Only consider alternatives if this method is a proven bottle-neck.