Idiom To Close A Cursor

Which of the following two should I be using to make sure that all the cursors are closed? Cursor c = getCursor(); if(c!=null && c.getCount()>0){ try{

Solution 1:

Neither, but the second one was closest.

• Option 1 doesn't properly close the Cursor when getCount() == 0
• Option 2 leaves the finally block exposed to a null pointer exception

I would use:

Cursorc= getCursor(); 
try { 
    if(c!=null && c.getCount()>0){ 
         // do stuff with the cursor
    }
}
catch(..) {
    //Handle ex
}
finally { 
    if(c != null) {
        c.close(); 
    }
}

... or if you expect the cursor to be null frequently, you could turn it on its head a little bit:

Cursorc= getCursor(); 
if(c != null) {
    try { 
        if(c.getCount()>0) { 
             // do stuff with the cursor
        }
    }
    catch(..) {
        //Handle ex
    }
    finally { 
        c.close(); 
    }
}

Solution 2:

This is even better:

• does not use c.getCount() - counting might require extra work for the database and is not needed
• initialize the cursor before the query block, so failure to create the query is not followed by the finally block

The code:

Cursorc= query(....);
if (c != null) {
   try {        
       while (c.moveToNext()) {  // If empty or after last record it returns false.    // process row...
       }
   } 
   finally {
       c.close();
    }
}

Note that c might be null in case of error or empty cursor. See https://stackoverflow.com/a/16108435/952135. I would report null return value in case of empty cursor as a bug, though.

Solution 3:

Best practice is the one below:

Cursorc=null;    
try {        
   c = query(....);      
   while (c.moveToNext()) {  // If empty or next to last record it returns false.    // do stuff..       
   }
} finally {
   if (c != null && !c.isClosed()) {  // If cursor is empty even though should close it.       
   c.close();
   c = null;  // high chances of quick memory release.
}

Solution 4:

Depends on what you're catching, but I'd say the second one, just in case c.getCount() throws an exception.

Also, some indentation wouldn't go amiss :)

Solution 5:

I'd say the first one, mainly because the second one will try to call c.close() even if c is null. Also, according to the docs, getCount()doesn't throw any exceptions, so there's no need to include it in the try block.