Is it good idea to close db in Insert method of db helper?

Question

Look at my DbHelper class. I think db.close() command is producing some errors in commands.
In every method that I've used getReadable Or GetWritable I've tried this.close();

This is my DbHelper:

 public class DbHelper extends SQLiteOpenHelper{

private SQLiteDatabase db;

public DbHelper(Context context) {
    super(context, "shareholders.db", null, 1);
}

@Override
public void onCreate(SQLiteDatabase db) {
    try {
        this.db = db;
        String sql = "CREATE TABLE IF NOT EXISTS news (id integer,title text,description text,sDate text)";
        db.execSQL(sql);


    } catch (Exception e) {
        xLog.error(e.getMessage());
    }


    ContentValues cv = new ContentValues();
    cv.put("name", "Username");
    cv.put("value", "default");
    db.insert("settings", null, cv);

    cv.clear();
    cv.put("name", "Password");
    cv.put("value", "default");
    db.insert("settings", null, cv);

    cv.clear();
    cv.put("name", "PersonId");
    cv.put("value", "default");
    db.insert("settings", null, cv);

    cv.clear();
    cv.put("name", "picture");
    cv.put("value", "");
    db.insert("settings", null, cv);
}

@Override
public void onUpgrade(SQLiteDatabase arg0, int arg1, int arg2) {
    // TODO Auto-generated method stub

}

public long insert(String table,ContentValues cv){
    SQLiteDatabase mydb =this.getWritableDatabase();
    long result=-1;
    try {
        result = mydb.insert(table,null, cv);
        }catch (Exception e) {
        xLog.error(e.getMessage());
    }
    this.close();
    return result;
}

public Cursor selectAll(String table){
    SQLiteDatabase mydb =this.getReadableDatabase();
    String sql = "SELECT * FROM "+table;
    xLog.info(sql);
    Cursor result=null;
    try {
        result = mydb.rawQuery(sql, null);
    } catch (Exception e) {
        xLog.error(e.getMessage());
    }
    this.close();
    return result;
}

public Cursor select(String table,String where){
    SQLiteDatabase mydb =this.getReadableDatabase();
    String sql = "SELECT * FROM "+table+" WHERE "+where;
    xLog.info(sql);

    Cursor result=null;
    try {
        result = mydb.rawQuery("SELECT * FROM "+table+" WHERE "+where, null);
    } catch (Exception e) {
        xLog.error(e.getMessage());
    }
    this.close();
    return result;
}

public long delete(String table,String condition){
    SQLiteDatabase mydb =this.getWritableDatabase();
    long result = -1;
    try {
        result = mydb.delete(table, condition, null);
    } catch (Exception e) {
        xLog.error(e.getMessage());
    }
    this.close();
    return result;
}

protected long empty(String table){
    SQLiteDatabase mydb =this.getWritableDatabase();
    long result = -1;
    try {
        result = mydb.delete(table, "", null);
    } catch (Exception e) {
        xLog.error(e.getMessage());
    }
    this.close();
    return result;
}

public long update(String table,ContentValues cv,String condition){
    SQLiteDatabase mydb =this.getWritableDatabase();
    long result = -1;
    try {
        result = mydb.update(table, cv, condition, null);
    } catch (Exception e) {
        xLog.error(e.getMessage());
    }
    this.close();
    return result;
}

protected void drop(String table){
    //TODO Produces a damn error!
    db.execSQL("DROP TABLE IF EXISTS "+table);
}

}

And this is my log cat:

11-26 13:12:20.738: W/System.err(2238): org.xmlpull.v1.XmlPullParserException: unexpected type (position:END_DOCUMENT null@1:0 in java.io.InputStreamReader@43e80ca8) 
11-26 13:12:20.738: W/System.err(2238):     at org.kxml2.io.KXmlParser.exception(KXmlParser.java:273)
11-26 13:12:20.738: W/System.err(2238):     at org.kxml2.io.KXmlParser.nextTag(KXmlParser.java:1420)
11-26 13:12:20.738: W/System.err(2238):     at org.ksoap2.SoapEnvelope.parse(SoapEnvelope.java:126)
11-26 13:12:20.738: W/System.err(2238):     at org.ksoap2.transport.Transport.parseResponse(Transport.java:96)
11-26 13:12:20.738: W/System.err(2238):     at org.ksoap2.transport.HttpTransportSE.call(HttpTransportSE.java:189)
11-26 13:12:20.745: W/System.err(2238):     at org.ksoap2.transport.HttpTransportSE.call(HttpTransportSE.java:95)
11-26 13:12:20.745: W/System.err(2238):     at ClassLibrary.WebService.CallMethod(WebService.java:49)
11-26 13:12:20.745: W/System.err(2238):     at ClassLibrary.AsyncCallWs.doInBackground(AsyncCallWs.java:64)
11-26 13:12:20.745: W/System.err(2238):     at ClassLibrary.AsyncCallWs.doInBackground(AsyncCallWs.java:1)
11-26 13:12:20.745: W/System.err(2238):     at android.os.AsyncTask$2.call(AsyncTask.java:185)
11-26 13:12:20.745: W/System.err(2238):     at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:305)
11-26 13:12:20.745: W/System.err(2238):     at java.util.concurrent.FutureTask.run(FutureTask.java:137)
11-26 13:12:20.745: W/System.err(2238):     at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1068)
11-26 13:12:20.745: W/System.err(2238):     at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:561)
11-26 13:12:20.745: W/System.err(2238):     at java.lang.Thread.run(Thread.java:1096)

Read http://stackoverflow.com/questions/4842112/when-to-close-db-connection-on-android-every-time-after-your-operation-finished — Pankaj Kumar, Nov 26 '13 at 09:55
Yes as Geralt mentioned, it will be a better practice to have only one instance of helper class or db for ur whole application. — Tamilselvan Kalimuthu, Nov 26 '13 at 10:05
@Tamilselvan How to change my class that way? possible to help me? — ehsan shirzadi, Nov 26 '13 at 10:34
create an application class and create public static instance for the db helper class in that application class itself and use this instance in all the activity classes. — Tamilselvan Kalimuthu, Nov 26 '13 at 10:38
first of all to insert value get the writable database . SQLiteDatabase database = this.getWritableDatabase(); and then insert — Tamilselvan Kalimuthu, Nov 26 '13 at 10:39

score 2 · Accepted Answer · answered Nov 26 '13 at 10:34

The exception is not related to the code but there is a problem:

this.db = db;

You're storing a reference to SQLiteDatabase you didn't obtain yourself with e.g. getReadableDatabase() or getWritableDatabase().

protected void drop(String table){
    //TODO Produces a damn error!
    db.execSQL("DROP TABLE IF EXISTS "+table);
}

Here you are using the stored reference which points to a database connection that is likely already closed. Use getWritableDatabase() here to get the db reference. Also, remove the SQLiteDatabase member variable. You don't need it anyway.

Internally SQLiteDatabase connections are reference counted. If the database is already open, opening it again just increments the reference count of the existing connection. Calling close() will decrement the reference counter and once it reaches exactly zero, the database is closed.

The database reference passed to onCreate() is opened and closed by the Android database framework.

Is it good idea to close db in Insert method of db helper?

1 Answers1