0

This question is a follow up question to this:

When should I close a cursor and db?

I'm learning how to use SQLite for Android and I'm using this tutorial for examples:

http://www.androidhive.info/2011/11/android-sqlite-database-tutorial/

I don't see any consistency as when they close the db or cursor. For example there's this method which closes the db:

public void addContact(Contact contact) 
{
SQLiteDatabase db = this.getWritableDatabase();

ContentValues values = new ContentValues();
values.put(KEY_NAME, contact.getName()); // Contact Name
values.put(KEY_PH_NO, contact.getPhoneNumber()); // Contact Phone Number

// Inserting Row
db.insert(TABLE_CONTACTS, null, values);
db.close(); // Closing database connection
}

But right after it there's this method which does not close it:

public Contact getContact(int id) {
SQLiteDatabase db = this.getReadableDatabase();

Cursor cursor = db.query(TABLE_CONTACTS, new String[] { KEY_ID,
        KEY_NAME, KEY_PH_NO }, KEY_ID + "=?",
        new String[] { String.valueOf(id) }, null, null, null, null);
if (cursor != null)
    cursor.moveToFirst();

Contact contact = new Contact(Integer.parseInt(cursor.getString(0)),
        cursor.getString(1), cursor.getString(2));
// return contact
return contact;
}

Same thing for closing the cursor.

Why was it correct to close the db after adding a contact but not after getting one? Is it a good practice to always close the db after calling getWritableDatabase() or getReadableDatabase()? Also, if I want to add several items to the DB, is it a good practice to keep the connection to the DB open and insert many items one by one, or should I somehow insert them all togather with a single query?

I have also read that you suppose to call getWritableDatabase() or getReadableDatabase() with an AsyncTask or IntentService, but the methods in the tutorial use it serially. Which is a better practice?

Community
  • 1
  • 1
CodeMonkey
  • 11,196
  • 30
  • 112
  • 203
  • 3
    First androidhive is a bad tutorial source. about closing SQLiteDatabase: generally if you not using different proccesses you shouldn't wory about closing db at all ... about closing Cursors: you should close it after you use it (only problem is when you are using CursorAdapter, then you should use old manageQuery(deprecated but still) or better: use new Loaders Api - both methods will take care of closing cursor) ... – Selvin Jul 20 '15 at 10:37
  • could you extend it a bit and write it as an answer instead of a comment? – CodeMonkey Jul 20 '15 at 10:43
  • as you are asking about Cursor too, so it is not a duplicate, but here is similar question about closing database: http://stackoverflow.com/questions/6608498/best-place-to-close-database-connection – Selvin Jul 20 '15 at 11:03

1 Answers1

0

Yes should always close a database connection after using it.

Writable database connections are more vulnerable as some other process may access the unclosed database connection and make changes to the database. which they cannot do through unclosed readable database connection.

Memory Leak should be minimized. I just answered a similar question yesterday. Here it is :

Memory leak in Java is a situation where some objects are not used by the application any more, but Garbage Collection fails to recognize them as unused and so, does not clean it up.

Every time you create an object, some space in memory is reserved for that object. And the same is for any Database Connection. So, after using the connection, if you dont close it, the GC doesnt get to know that this object will not be used anymore and hence does not delete it. So, it stays there in memory, eating valuable resource while the rest of the program runs. [Hence resource leak].

This is not at all desired. Furthermore, this exposes your program to security issues, as the connection is open, it is vulnerable, and changes can be made to the database. Remember, Even after the Activity closes, if there is a running thread, or an AsyncTask, the database connection remains open as well along with the thread.

Further reading : What is a “Memory leak” in Java?

From StackOverflow : Closing database connection to avoid memory leak

Community
  • 1
  • 1
Samrat Dutta
  • 1,727
  • 1
  • 11
  • 23
  • So you're saying I could just open the database once at onCreate for example and just leave it open through the entire app's life time until onDestroy is closed and only then close it? – CodeMonkey Jul 20 '15 at 10:46
  • If you use the database connection quite extensively throughout the app, then yes. that should be the method used. Because creating Database connections is very costly. And should not be repeated. But in case, u require it only a few times in the app, then you can close the connection in the method itself. – Samrat Dutta Jul 20 '15 at 10:48
  • best practices is to use ContentProvider and not close the DB at all ... there should be only 1 connection ... if you don't wana follow best practices you can use singleton which will keep only 1 connection to the db ... and still you can't have a proper event to close it (as onDestroy may not be called) – Selvin Jul 20 '15 at 10:51
  • is it logical to add 2 database vars as class members, one for the writable and one for the readable and open them? since there is for some reason a writable and readable dbs. – CodeMonkey Jul 20 '15 at 10:52
  • 1
    I dont think that will be logical. Because, the writable database can read as well, so, you can use that to read and write both. :) – Samrat Dutta Jul 20 '15 at 10:55
  • @SamratDutta you are wrong ... it is not a leak ... here is what wrote a Google engineer: https://groups.google.com/forum/#!msg/android-developers/nopkaw4UZ9U/cPfPL3uW7nQJ – Selvin Jul 20 '15 at 10:58
  • @Selvin so what do you think is the best practice in my case (could be in a different answer). Isn't a ContentProvider used only for sharing data with other apps? I just want to store data in the local device DB and that's it. – CodeMonkey Jul 20 '15 at 11:00
  • I read the doc, and that is exactly what I meant. If you have read my answer. I said, it is a leak when you do not close the connection after the application has finished using the resource. And that is not a good thing. Ofcourse the resource will be cleaned up when the application shuts down. But if you need the database connection only once, say at the start of the application, there is no point in keeping it open. As this leads to vulnerabilities. Malicious background processes may jump on it and use the open connection to make changes. Atleast, that is what I have learnt. – Samrat Dutta Jul 20 '15 at 11:09
  • Do you might also have an answer for the asynctask question? – CodeMonkey Jul 20 '15 at 11:18
  • Usually I use AsyncTask to make data transactions over internet. Or in association with BroadcastReceiver. You can make database transactions in AsyncTask too ofcourse. What is better depends upon the situation. If is perfectly fine to use both methods. – Samrat Dutta Jul 20 '15 at 11:23