Null checking in nested getter calls [closed]

Question

I have the following piece of code and I would like to see what different approaches would be to solve that in a more elegant way.

The thing is that I don't know whether is better to have the null checks or the try/catch block there or maybe the code below should not happen in the first place. Maybe one can give another solution so that I can get the thumb URL from a images list that is set on a people object without having to check for null on each getter call.

if (people.getImages() != null && people.getImages().size() > 0 && people.getImages().get(0).getUrl() != null) {
    Url thumbUrl = people.getImages().get(0).getUrl().getThumb();
    // do something with the thumbUrl
}

Or I can have:

try {
    Url thumbUrl = people.getImages().get(0).getUrl().getThumb();
    // do something with the thumbUrl
} catch (Exception e) {
    // handle the null pointer exception
}

EDIT

For more information, I am interested more in knowing if there any other ways to avoid calling getter methods one after another until I get my desired data from an object and if the above code is good enough anyway.

The code works ok in both cases but I don't like to have the multiple null checks in that if statement and also catching exceptions is not a good solution either.

The getImages() method returns a list of Image objects and getThumb() returns an Url object that I need in order to fetch an image over the internet.

Answer 1

Let's count the number of times you're writing get.

if (people.getImages() != null && people.getImages().size() > 0 && people.getImages().get(0).getUrl() != null) {
    Url thumbUrl = people.getImages().get(0).getUrl().getThumb();
    // do something with the thumbUrl
}

5 times inside the if-condition and 4 times inside the block. That's a bit too much. Remember the Law of Demeter.

In your case I would do:

if (people.hasImage(0)) {
     Url thumbUrl = people.getImage(0);
}

The idea is to hide some logic inside the people object.

It also seems like people.getImages() is a Collection, which makes me think that it is a bad idea to ever let that be null. An empty collection is better than a null collection.

As for your main question, I believe Heslacher says it all:

An avoided exception is always better then a handled exception

This applies especially to NullPointerException and ArrayIndexOutOfBoundsException (and all other RuntimeExceptions).

Also, you should always catch the most-specific Exception possible. Never catch Exception. But then again, never catch NullPointerException. Fix your code to not cause one in the first place.

Related previous Code Review answers about exceptions:

Rethrowing exception just as an information

https://codereview.stackexchange.com/questions/45637/exception-handling-with-instanceof-rather-than-catch/45639#45639