madcoderz
madcoderz

Reputation: 4431

trouble finding NullPointerException used try catch instead of if statement

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.

Upvotes: 1

Views: 110

Answers (2)

Rohit Jain
Rohit Jain

Reputation: 213243

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 2nd condition is not evaluated, and hence no NullPointerException. This is how short-circuit OR operator works. It only evaluates the 2nd expression, if 1st one evaluates to false.
  • If null check fails, then 2nd 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 2nd 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
}

Upvotes: 5

dcampoy
dcampoy

Reputation: 33

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?

Upvotes: 1

Related Questions