Returning sqlite query as cursor or object/list

Question

When I started my app years ago, I did some tutorials and always did my queries to the database returning the cursor (without closing it):

public Cursor querySingleId(String szId) {
    SQLiteDatabase db = getWritableDatabase();
    return db.query(TABLE_ADR, szGetTableEntries, _ID + " = ?", new String[]{szId}, null, null, null);
}   

Now I am refactoring my code to MVVM and added models, so I changed my code to this:

public Card querySingleId(String szId) {
    SQLiteDatabase db = getWritableDatabase();
    Cursor c1 = db.query(TABLE_ADR, szGetTableEntries, _ID + " = ?", new String[]{szId}, null, null, null);
    c1.moveToFirst();

    return new Card(c1.getString(c1.getColumnIndex(DbHandler.NAME)),
                    c1.getString(c1.getColumnIndex(DbHandler.STREET)),
                    c1.getString(c1.getColumnIndex(DbHandler.CITY)));
}

I read that cursors should always be closed (memory leak). Which is the best/most conform approach to return my data from the database? I'm also unsure if there are multiple results, should I stay with a cursor or change to a list?

public List<Card> queryAll() {
    SQLiteDatabase db = getWritableDatabase();
    Cursor c1 = db.query(TABLE_ADR, szGetTableEntries, null, null, null, null, null);
    List<Card> list = new ArrayList<>();

    if(c1.moveToFirst()){
        do{
            list.add(new Card(c1.getString(c1.getColumnIndex(DbHandler.NAME)),
                              c1.getString(c1.getColumnIndex(DbHandler.STREET)),
                              c1.getString(c1.getColumnIndex(DbHandler.CITY))));
        } while(c1.moveToNext());
    }
    c1.close();
    return list;
}

Is this all just a matter of taste or are there reasons why it should return a cursor or a list/object? Depending where in my code I need the data, a list or a cursor is more convenient. I'm just not sure what is the correct approach in sqlite queries. There are so many code examples and but it seems most is copy/paste without really digging into the topic.

Answer 1

If the query can return multiple rows then you should return a list.

If you are sure that the query will return just a single Card then returning that single Card would be OK (probably preferable) BUT you should close the Cursor.

• However, there isn't an actual requirement/need to do so (e.g. if you your initial activity uses a Cursor for a ListView/Spinner then you may not want to close the Cursor but reuse it and use the adapter's swapCursor when the Activity resumes). The cursor would be effectively closed, as would the database when the App finishes.

• As you have used the column _ID which is typically used for a column that is an alias of the rowid column, which is typically generated by SQLite then if used/defined as such (column has been defined explicitly or implicitly as INTEGER PRIMARY KEY with or without AUTOINCREMENT) then it will be a unique value and only return a single row as you have _ID = ?. As such there is a high likeliehood that a single row, or no row would be returned, and unlikely that multiple rows are returned.

So (for a single Card):-

public Card querySingleId(String szId) {
    SQLiteDatabase db = getWritableDatabase();
    Cursor c1 = db.query(TABLE_ADR, szGetTableEntries, _ID + " = ?", new String[]{szId}, null, null, null);
    c1.moveToFirst();

    return new Card(c1.getString(c1.getColumnIndex(DbHandler.NAME)),
                    c1.getString(c1.getColumnIndex(DbHandler.STREET)),
                    c1.getString(c1.getColumnIndex(DbHandler.CITY)));
}

Should be something like :-

public Card querySingleId(String szId) {
    SQLiteDatabase db = getWritableDatabase();
    Card rv = null;
    Cursor c1 = db.query(TABLE_ADR, szGetTableEntries, _ID + " = ?", new String[]{szId}, null, null, null);
    if (c1.moveToFirst()) { //<<<<<< Should always check if the move moved 
        rv = new Card(c1.getString(c1.getColumnIndex(DbHandler.NAME)),
            c1.getString(c1.getColumnIndex(DbHandler.STREET)),
            c1.getString(c1.getColumnIndex(DbHandler.CITY)));
    }
    c1.close();
    return rv; //<<<<< Note should check the returned Card for null
}

In addition to memory leaks not closing Cursors can result in a too many open connections (1K (1024) if memory serves correctly) and then a exception: unable to open database file (code 14); as underneath all the wrappers a Cursor has a file associated with it.