VansFannel
VansFannel

Reputation: 45961

Software design: where to put database open and close

I'm developing an android 3.1 application.

This question is not specific for Android, it is about how to design a class that access a database. I asked here because my code is for Android.

I have a class, DBManager, to work with Sqlite database. This is a part of its implementation:

public class DBManager
{
    // Variable to hold the database instance
    private SQLiteDatabase db;
    // Database open/upgrade helper
    private DatabaseHelper dbHelper;

    public DBManager(Context _context)
    {
        Log.v("DBManager", "constructor");
        dbHelper = new DatabaseHelper(_context, SqlConstants.DATABASE_NAME, null, SqlConstants.DATABASE_VERSION);
    }

    public DBManager open() throws SQLException
    {
        Log.v("DBManager", "open");
        db = dbHelper.getWritableDatabase();
        return this;
    }

    public void close()
    {
        Log.v("DBManager", "close");
        db.close();
    }

        ...

        /**
     * Query all forms available locally.
     * @return A list with all forms (form.name and form.FormId) available on local db
     * or null if there was a problem.
     */
    public ArrayList<Form> getAllForms()
    {
        Log.v("DBManager", "getAllForms");
        ArrayList<Form> list = null;
        Cursor c = null;

        try
        {
            c = this.getAllFormsCursor();
            if (c != null)
            {
                int formNameIndex = c.getColumnIndex(SqlConstants.COLUMN_FORM_NAME);
                int formIdIndex = c.getColumnIndex(SqlConstants.COLUMN_FORM_ID);

                c.moveToFirst();
                if (c.getCount() > 0)
                {
                    list = new ArrayList<Form>(c.getCount());
                    do
                    {
                        Form f = new Form();
                        f.Name = c.getString(formNameIndex);
                        f.FormId = c.getString(formIdIndex);
                        list.add(f);
                    }
                    while (c.moveToNext());
                }
            }
        }
        catch (Exception e)
        {
            e.printStackTrace();
            list = null;
        }
        finally
        {
            if (c != null)
                c.close();
        }
        return list;
    }

    private Cursor getAllFormsCursor()
    {
        Log.v("DBManager", "getAllFormsCursor");

        return db.query(SqlConstants.TABLE_FORM,
                new String[] {
                SqlConstants.COLUMN_FORM_ID,
                SqlConstants.COLUMN_FORM_NAME}, null, null, null, null, null);
    }
}

And this is an AsyncTask that uses DBManager:

private class DbFormListAsyncTask extends AsyncTask<Void, Void, ArrayList<Form>>
{
    private Context mContext;
    private ProgressDialog loadingDialog;
    private DBManager dbMan;

    DbFormListAsyncTask(Context context)
    {
        this.mContext = context;
        loadingDialog = new ProgressDialog(mContext);
        loadingDialog.setProgressStyle(ProgressDialog.STYLE_SPINNER);
        loadingDialog.setMessage("Retriving forms. Please wait...");
        loadingDialog.setCancelable(false);
        loadingDialog.show();
    }

    @Override
    protected ArrayList<Form> doInBackground(Void... arg0)
    {
        dbMan = new DBManager(mContext);
        dbMan.open();

        return dbMan.getAllForms();
    }

    protected void onPostExecute(ArrayList<Form> forms)
    {
        if (forms != null)
        {
            ListActivity act = (ListActivity) mContext;
            act.setListAdapter(new AvaFormAdapter(act, R.layout.ava_list_item, forms));
        }
        else
        {
            TextView errorMsg = (TextView)
                    ((FormsListActivity) mContext).findViewById(R.id.formErrorMsg);
            errorMsg.setText("Problem getting forms. Please try again later.");
        }
        loadingDialog.dismiss();
        if (dbMan != null)
            dbMan.close();
    }
}

As you can see I have to:

  1. Create DBManager instance.
  2. Open database with dbMan.open()
  3. Call dbMan.getAllForms()
  4. Close database with dbMan.close() on onPostExecute.

I thought that I could add db.open() and db.close() on dbMan.getAllForms() to avoid calling it every time I use dbMan.getAllForms().

What do you think about this? What is the best approach?

Upvotes: 0

Views: 264

Answers (2)

Nikolay Elenkov
Nikolay Elenkov

Reputation: 52956

Make your database helper class a singleton, and don't explicitly close the SQLiteDatabase. It will be closed and flushed when your app's process exits.

Upvotes: 0

zapl
zapl

Reputation: 63955

I would put it inside getAllForms() or do something like that

protected ArrayList<Form> doInBackground(Void... arg0)
{
    dbMan = new DBManager(mContext);
    dbMan.open();
    ArrayList<Form> resutl = dbMan.getAllForms();
    dbMan.close();
    return result;
}

Since you don't need the db connection after you have the result you can close it right away.

Edit: if you run that AsyncTask several times then opening/closing will introduce unnecessary overhead. In that case you may want to instanciate the dbManager from your Activity (maybe open() in the constructor of DbManager) and close it once you leave your activity. Then pass Dbmanager to the AsyncTask.

Upvotes: 0

Related Questions