0

For example consider this function

public void insertRow(CoolObject coolObject) {
    ContentValues values = new ContentValues();
    values.put("some field name", coolObject.getSomeValue()); 
    //and so on and so on

    mDatabase.open();
    mDatabase.insertWithOnConflict("CoolTable", 
        null, 
        values, 
        SQLiteDatabase.CONFLICT_IGNORE);
    mDatabase.close();
}

I open the database, do an SQL command of some kind using the database, then close the database again.

Now consider this:

public void insertRows(List<CoolObject> coolObjectList) {
    mDatabase.beginTransaction();
    try {
        for (CoolObject coolObject : coolObjectList) {
            insertRow(coolObject);
        }
        mDatabase.setTransactionSuccessful();
    }
    finally {
        mDatabase.endTransaction();
    }
}

In this case I run a loop that calls insertRow() repeatedly, but within the framework of a single database transaction.

My question:

Is it a problem that I call open and close as I do? It may make sense for a single call, but I worry that if I am calling the insert row over and over again, that's a lot of opening and closing.

And if the database is closed by the time I finish the row operation, do I have to open it again even for the things like mDatabase.beginTransaction(); or mDatabase.setTransactionSuccessful(); or mDatabase.endTransaction();?

I also can't just take the open and close out of the insertRow() either and add it to the start and end of the insertRows() method either because then it's, well, not in insertRow() now so if I want to call that function by itself I am no longer opening/closing.

What's the accepted way to get around these errors?

KaliMa
  • 1,970
  • 6
  • 26
  • 51
  • "Is it a problem that I call open and close as I do?" -- I wouldn't do it that way. Open it once. Close it... probably never. We discussed some of this [a few months ago](https://stackoverflow.com/questions/36730859/correct-way-to-open-close-the-database). – CommonsWare Sep 27 '16 at 00:10
  • But then how can you guarantee the database is open when you are calling `insertRow()`? (say something goes wrong so that the database is actually not open/available by the time `insertWithOnConflict()` is invoked) – KaliMa Sep 27 '16 at 00:12
  • "But then how can you guarantee the database is open when you are calling insertRow()?" -- if you are using `SQLiteOpenHelper`, call `getReadableDatabase()`. That will open the database if it is not already open. If you are not using `SQLiteOpenHelper`, roll your own lazy-initialization approach. And in your particular case, you know whether it is open before calling `insertRow()`, as it has to be open for `beginTransaction()`. – CommonsWare Sep 27 '16 at 00:15
  • My helper already does this with `public void open() throws SQLException { mDatabase = mSQLHelper.getWritableDatabase(); }` -- but I am asking more specifically about how to error-handle if we're assuming that calling operations on `mDatabase` won't result in errors – KaliMa Sep 27 '16 at 00:17
  • In your first code you open the connection use it to make insertions and then do some operations and close it. If there was some error with your insertion then it would throw error and in the process not close the connection. Soon you would run out of connection pools. In the second one you are doing multiple inserts one at a time. You could possibly achieve this by making a bulk insertion? Generally a bulk insertion is better than multiple insertions in terms of performance. – Deepak Puthraya Sep 27 '16 at 06:49
  • @Putty Using what syntax? As far as I could tell this *was* a bulk insert (source: http://stackoverflow.com/a/32088155/1855273) – KaliMa Sep 27 '16 at 08:21
  • @KaliMa What you are doing in your first example is bulk insertion in single query and in the second one you are inserting multiple times with one connection. If I were to improve on your code I would use the first code example and wrap your insert statement in `try{} catch(){} finally{}` – Deepak Puthraya Sep 27 '16 at 09:10

2 Answers2

0

Is it a problem that I call open and close as I do? It may make sense for a single call, but I worry that if I am calling the insert row over and over again, that's a lot of opening and closing.

Depending on database, you will open a new process or thread each time, which not not really efficient, combined to the fact that it may not close properly...

Try combined inserts, it will help staying low on resources.

If you have an opportunity and still want to go the first way, implement a connection pool that will keep connections opened.

Nenelf
  • 1
  • 1
0
public void insertRow(CoolObject coolObject) {
    ContentValues values = new ContentValues();
    values.put("some field name", coolObject.getSomeValue()); 
    //and so on and so on

    mDatabase.open();
    try {
        // Do whatever you want to do with your connection
        mDatabase.insertWithOnConflict("CoolTable", 
            null, 
            values, 
            SQLiteDatabase.CONFLICT_IGNORE);
    }catch(Exception err) {
        // handle error
    }finally {
        mDatabase.close();        
    }
}

In my opinion the above code should let you handle most cases. In the above method if the mDatabase.open() were to throw error it would be a problem. To handle that case you could pass the connection as a resource parameter to your try method.

Deepak Puthraya
  • 1,325
  • 2
  • 17
  • 28