Android SQLite database close in query()

Question

I see ContentProvider.query() returns a Cursor object. Usually the cursor is the result of SQLiteDatabase query. As the following code snippet:

public Cursor query() {     
        try {
            SQLiteDatabase db = this.getReadableDatabase(); 
            Cursor c = db.query(Cfg.table_name, new String[] {"*"}, null, null, null, null, null);
            return c;
        }
        catch(Exception exp) {
        }

        return null;
    }

In the code, db is not closed. Is there any problem?

Answer 1

Are you trying to close the db when complete? If so try:

public Cursor query() {
        SQLiteDatabase db = null;   
        try {
            SQLiteDatabase db = this.getReadableDatabase(); 
            Cursor c = db.query(Cfg.table_name, new String[] {"*"}, null, null, null, null, null);
            return c;
        }
        catch(Exception exp) {
        }finally{
           try{if(null != db){ db.close(); db = null;}}catch(Exception e){}
        }

        return null;
    }

That should close the db properly everytime. If you are wondering why it's not closed, I would investigation what that catched Exception is saying.

By the way, I would replace "new String[] {"*"}" with null. http://developer.android.com/reference/android/database/sqlite/SQLiteDatabase.html

UPDATE: Okay now I understand the problem. You may need to restructure your class a bit though.

TLDR: Ideally for me, what you should end up would be two classes. One is for helping you open the DB. Second would be holding SQLiteDatabase object as a property. You may open the connection and initialize the DB object as soon as you create this class (or create method use for opening database). All methods will only use the db object for SCUD only. Then all db object closing or decomposing can be place in a method or override onDestroy. Example below but if you have time to spare this is a good tutorial http://www.vogella.de/articles/AndroidSQLite/article.html.

The first one is DBOpenHelper. The purpose of this class will be for managing tables and help you open database for modification.

package com.mondial.th.rsa.db;

import android.content.Context;
import android.database.sqlite.SQLiteDatabase;
import android.database.sqlite.SQLiteDatabase.CursorFactory;
import android.database.sqlite.SQLiteOpenHelper;
import android.util.Log;

/**
 * DBOpenHelper helps opening database and managing database's upgrade or creation.
 * @author Poohdish Rattanavijai
 *
 */
public class DBOpenHelper extends SQLiteOpenHelper {
    private static final String TAG = DBOpenHelper.class.getSimpleName();
    private static final int DATABASE_VERSION = 2;
    private static final String DATABASE_NAME = "rsa_db";

    public DBOpenHelper(Context context) {
        super(context, DATABASE_NAME, null, DATABASE_VERSION);
    }

    public DBOpenHelper(Context context, String name, CursorFactory factory,
            int version) {
        super(context, DATABASE_NAME, null, DATABASE_VERSION);
    }

    @Override
    public void onCreate(SQLiteDatabase database) {
        Log.d(TAG, "DBOpenHelper.onCreate");
        // Create table goes here.
    }

    @Override
    public void onUpgrade(SQLiteDatabase database, int oldVersion, int newVersion) {
        // Drop and recreate table goes here.
    }

}

The second class will handle all the query for you by using DBOpenHelper shown earlier.

import android.content.ContentValues;
import android.content.Context;
import android.database.Cursor;
import android.database.SQLException;
import android.database.sqlite.SQLiteDatabase;
import android.util.Log;

import com.mondial.th.rsa.vo.ProfileVO;

/**
 * Database adapter for managing Profile table
 * @author Poohdish Rattanavijai
 *
 */
public class ProfileDBAdapter {
    private static final String TAG = ProfileDBAdapter.class.getName();
    private Context context;
    private SQLiteDatabase db;
    private DBOpenHelper dbHelper;

    public ProfileDBAdapter(Context context){
        this.context = context;
    }

    /**
     * Open writable database.
     * @return writable SQLDatabase
     * @throws SQLException
     */
    public ProfileDBAdapter open() throws SQLException{
        Log.d(TAG, "Open ProfileDBAdapter");
        dbHelper = new DBOpenHelper(context);

        db = dbHelper.getWritableDatabase();
        return this;
    }

    /**
     * Close database; Exception omitted
     */
    public void close(){
        try {
            dbHelper.close();
        } catch (Exception e) {
            //e.printStackTrace();
        }
    }

    /**
     * Creating new record in profile table with given VO.
     * @param profile VO representing data in the new record.
     * @return the row ID of the newly inserted row, or -1 if an error occurred.
     */
    public long createProfile(ProfileVO profile){
        ContentValues values = createContentValues(profile);
        return db.insert(ProfileVO.TABLE_NAME, null, values);
    }

    /**
     * Updating record in profile table with given VO by using ID column.
     * @param profile VO representing new data to updated.
     * @return the number of rows affected .
     */
    public boolean updateProfile(ProfileVO profile){
        ContentValues values = createContentValues(profile);
        return db.update(ProfileVO.TABLE_NAME, values, ProfileVO.COLUMN_ID + "=" + profile.getId(), null) > 0;
    }

    /**
     * Deleting a row representing given VO off profile table by using ID column.
     * @param profile
     * @return
     */
    public boolean deleteProfile(ProfileVO profile){
        return deleteProfile(profile.getId());
    }

    /**
     * Deleting a row off profile table with given ID column.
     * @param profile
     * @return
     */
    public boolean deleteProfile(long rowId){
        return db.delete(ProfileVO.TABLE_NAME, ProfileVO.COLUMN_ID + "=" + rowId, null) > 0;
    }

    /**
     * open cursor representing every records in profile table.
     * @return Cursor representing every records in profile table.
     */
    public Cursor fetchAllProfiles(){
        return db.query(ProfileVO.TABLE_NAME, null, null, null, null, null, null);
    }

    /**
     * open cursor representing a row in profile table with given ID.
     * @return Cursor representing a row in profile table with given ID.
     */
    public Cursor fetchProfile(long rowId){
        Cursor mCursor = db.query(true, ProfileVO.TABLE_NAME, null, ProfileVO.COLUMN_ID + "=" + rowId
                , null, null, null, null, null);

        if(null != mCursor){
            mCursor.moveToFirst();
        }

        return mCursor;
    }

    /**
     * This class translate given VO into ContentValues for ContentResolver to parse data and talk to the database.
     * @param profile VO
     * @return ContentsValues containing given VO's data except ID.
     */
    private ContentValues createContentValues(ProfileVO profile){
        ContentValues values = new ContentValues();

        values.put(ProfileVO.COLUMN_DOB, profile.getDob());
        values.put(ProfileVO.COLUMN_FIRSTNAME, profile.getFirstname());
        values.put(ProfileVO.COLUMN_LASTNAME, profile.getLastname());

        return values;
    }
}

Then you have one perfect class that act as a DAO. For the above example the usage would be.

ProfileDBAdapter dbHelper = new ProfileDBAdapter(context);
dbHelper.open();
cursor = dbHelper.fetchAllProfiles();
if(cursor.getCount() > 0){
 //TODO Data exists, do stuff.
}
try {
    if(null != cursor && !cursor.isClosed()){
       cursor.close();
    }
} catch (Exception e) {
    e.printStackTrace();
}

if(null != dbHelper){
    dbHelper.close();
    dbHelper = null;
}

Answer 2

Yes, I believe there is a problem. You could pass 'db' as an argument then the caller can close it when done with the cursor.