trouble finding NullPointerException used try catch instead of if statement

Question

I've been struggling to find why my if statement didnt work properly so I used a try catch block instead. This is the if statement as I had it:

//selectArtistByName returns an Artist object
if (!selectArtistByName(artist.getName()).equals(artist.getName()) || 
    selectArtistByName(artist.getName())==null) {
    //save data to database
}

When I ran the above, I got a NullPointerException because the method selectArtistByName was returning null as the database was empty. What I don't understand is why it didn't go in the if statement when I was getting null. So I did this and it worked:

try {
    if (!selectArtistByName(artist.getName()).equals(artist.getName())) {
    }
} catch (NullPointerException e) {
    m_db.insert(TABLE_ARTIST, null, artistContents);
}

I'm not a Java guru but it looks like a horrible fix to me. How could I fix this.

Rohit Jain · Answer 1 · 2013-08-04T16:54:57.337

You just need to change the order of condition in if block:

if (selectArtistByName(artist.getName()) == null || 
   !selectArtistByName(artist.getName()).equals(artist.getName())) {
    //save data to database
}

Do the null check first.
If that succeeds, then 2^nd condition is not evaluated, and hence no NullPointerException. This is how short-circuit OR operator works. It only evaluates the 2^nd expression, if 1^st one evaluates to false.
If null check fails, then 2^nd condition is evaluated, which wouldn't throw NPE, as it has already been confirmed by first condition.

Also, as rightly pointed out by @ruakh in comment, your condition seems to be broken. selectArtistByName sounds to be returning an Artist, which you can't compare with String.

I guess, you don't even need the 2^nd condition. I would assume, selectArtistByName() method has already done the equality check for name, based on which it will return Artist. Just check that selectArtistByName method return null, that would be enough. So, you should change the if block to:

if (selectArtistByName(artist.getName()) == null) {
    //save data to database
}

+1, but it should be noted that there's another problem here: it's unlikely that `selectArtistByName(artist.getName()).equals(artist.getName())` will ever be true, since `selectArtistByName(artist.getName())` is an `Artist` and `artist.getName()` is presumably a `String`. (I'm not sure exactly what the condition is *supposed* to look like, though.) — ruakh, Aug 04 '13 at 16:52

score 1 · Answer 2 · edited May 23 '17 at 12:05

1

Just put the null condition check at the beginning to shortcut when artist is unknown:

if (selectArtistByName(artist.getName())==null || !selectArtistByName(artist.getName()).equals(artist.getName())) {
     //save data to database
}

You can find more info about lazy evaluation in this other question: Does Java have lazy evaluation?

edited May 23 '17 at 12:05

Community

1
1

answered Aug 04 '13 at 16:53

dcampoy

33
8

trouble finding NullPointerException used try catch instead of if statement

2 Answers2