Is this a correct way to do singleton for SqliteHelper in android?

Question

I've tried to write down a proper singleton access for SqliteHelper in android. Got bits and pieces from here and there to form the final version, but not sure if I'm missing something here.

public class DatabaseHelper extends SQLiteOpenHelper {

    private static final String DATABASE_NAME = "EffiaSoft";
    private static final int DATABASE_VERSION = 1;
    private static SQLiteOpenHelper sInstance;
    private Semaphore semaphore = new Semaphore(1);
    private Context mCxt;

    /**
     * Constructor should be private to prevent direct instantiation.
     * make call to static method "getInstance()" instead.
     */
    private DatabaseHelper(Context context) {
        super(context, DATABASE_NAME, null, DATABASE_VERSION);
        this.mCxt = context;
    }

    public static synchronized SQLiteOpenHelper getInstance(Context context) {
        if (sInstance == null) {
            sInstance = new DatabaseHelper(context.getApplicationContext());
        }
        return sInstance;
    }

    private static <T> void setFields(Cursor cursor, T returnObject, Field[] declaredFields, String[] cursorColumns) throws IllegalAccessException {
        for (Field declaredField : declaredFields) {
            String fieldName = declaredField.getName();
            boolean hasColumn = false;
            for (String columnName : cursorColumns) {
                if (columnName.equals(fieldName))
                    hasColumn = true;
                else
                    continue;
                break;
            }
            if (hasColumn) {
                declaredField.setAccessible(true);
                int columnIndex = cursor.getColumnIndex(fieldName);
                if (columnIndex >= 0) {
                    if (declaredField.getType() == Character.TYPE) {
                        String value = cursor.getString(columnIndex);
                        if (!ValidationHelper.isNullOrEmpty(value) && value.length() == 1) {
                            char c = value.charAt(0);
                            declaredField.set(returnObject, c);
                        }
                    } else if (declaredField.getType() == Short.TYPE) {
                        declaredField.set(returnObject, cursor.getShort(columnIndex));
                    } else if (declaredField.getType() == Integer.TYPE) {
                        declaredField.set(returnObject, cursor.getInt(columnIndex));
                    } else if (declaredField.getType() == Long.TYPE) {
                        declaredField.set(returnObject, cursor.getLong(columnIndex));
                    } else if (declaredField.getType() == Float.TYPE) {
                        declaredField.set(returnObject, cursor.getFloat(columnIndex));
                    } else if (declaredField.getType() == Double.TYPE) {
                        declaredField.set(returnObject, cursor.getDouble(columnIndex));
                    } else if (declaredField.getType() == Boolean.TYPE) {
                        String temp = cursor.getString(columnIndex);
                        declaredField.setBoolean(returnObject, temp.equalsIgnoreCase("Y"));
                    } else if (Date.class.equals(declaredField.getType())) {
                        if (!ValidationHelper.isNullOrEmpty(cursor.getString(columnIndex))) {
                            Date date = ValueFormatter.formatDateTime(cursor.getString(columnIndex));
                            declaredField.set(returnObject, date);
                        }
                    } else if (BigDecimal.class.equals(declaredField.getType())) {
                        declaredField.set(returnObject, BigDecimal.valueOf(cursor.getDouble(columnIndex)));
                    } else {
                        declaredField.set(returnObject, cursor.getString(columnIndex));
                    }
                }
            }

        }
    }

    private void Disconnect() {
        semaphore.release();
    }

    private void Connect() {
        try {
            semaphore.acquire();
        } catch (InterruptedException e) {
            LogHelper.writeExceptionLog(e);
        }
    }

    @Override
    public void onCreate(SQLiteDatabase db) {

    }

    public <T> ArrayList<T> getData(Context context, String tableName, String columnName, String filterCondition, String orderBy,
                                    String groupBy, Class<T> type, SQLiteDatabase database) {

        Connect();
        ArrayList<T> arrayList = null;
        try {
            if (context != null && !ValidationHelper.isNullOrEmpty(tableName)) {
                SQLiteDatabase db;
                if (database == null) {
                    db = getWritableDatabase();
                } else
                    db = database;
                arrayList = new ArrayList<>();
                if (db != null) {
                    StringBuilder selectQuery = new StringBuilder();
                    selectQuery.append("SELECT ");
                    if (columnName != null && columnName.length() > 0)
                        selectQuery.append(columnName);
                    else
                        selectQuery.append("*");
                    selectQuery.append(" FROM ");
                    selectQuery.append(tableName);
                    if (!ValidationHelper.isNullOrEmpty(filterCondition)) {
                        selectQuery.append(" WHERE ");
                        selectQuery.append(filterCondition);
                    }
                    if (!ValidationHelper.isNullOrEmpty(orderBy)) {
                        selectQuery.append(" ORDER BY ");
                        selectQuery.append(orderBy);
                    }
                    if (!ValidationHelper.isNullOrEmpty(groupBy)) {
                        selectQuery.append(" GROUP BY ");
                        selectQuery.append(groupBy);
                    }
                    Cursor cursor;
                    cursor = db.rawQuery(selectQuery.toString(), null);
                    if (cursor != null) {
                        String[] cursorColumn = cursor.getColumnNames();
                        if (cursor.moveToFirst()) {
                            do {
                                T returnObject = type.newInstance();
                                Class<?> returnClass = returnObject.getClass();
                                if (!returnClass.getSuperclass().equals(Object.class)) {
                                    setFields(cursor, returnObject, returnClass.getSuperclass().getDeclaredFields(), cursorColumn);
                                }
                                setFields(cursor, returnObject, returnClass.getDeclaredFields(), cursorColumn);
                                arrayList.add(returnObject);
                            } while (cursor.moveToNext());
                        }

                    }
                }
            }
        } catch (Exception e) {
            LogHelper.writeExceptionLog(e);
        } finally {
            Disconnect();
        }
        return arrayList;
    }

    @Override
    public void onUpgrade(SQLiteDatabase db, int oldVersion, int newVersion) {

    }

    @Override
    protected void finalize() throws Throwable {
        this.close();
        super.finalize();
        Disconnect();
    }
}

It is not appropriate to cache a single instance based on a parameter, as you do in `getInstance(Context)`. What if you call it with a different parameter? You should always get back an instance appropriate for the parameter. Ask yourself why you really need this to be a singleton. — Andy Turner, Apr 08 '16 at 07:32
We have extreme multi threaded access with transactions and it's quite often we receive the error `cannot open an already closed database`. — Soham Dasgupta, Apr 08 '16 at 07:37
Try one of this singletons [singleton pattern](http://stackoverflow.com/a/22497718/3436179) — Alexander, Apr 08 '16 at 08:02
Yup - the method you're using is [fine](http://www.androiddesignpatterns.com/2012/05/correctly-managing-your-sqlite-database.html) — PPartisan, Apr 08 '16 at 10:03
@AndyTurner What do you mean by not appropriate to cache the instance? How would you alter the singleton pattern to avoid this? — Prof, Feb 17 '19 at 17:14

Bui Quang Huy · Answer 1 · 2016-04-08T09:40:52.620

0

You just need to define like this:

private static DatabaseHelper mInstance = null;

public static DatabaseHelper getInstance(Context context) {
        if (mInstance == null) {
            mInstance = new DatabaseHelper(context);
        }
        return mInstance;
   }

In your activity:

DatabaseHelper db = DatabaseHelper.getInstance(context);

edited Apr 08 '16 at 09:40

answered Apr 08 '16 at 07:39

Bui Quang Huy

1,784
2
17
50

1

besides, maybe you need to define database = getWritableDatabase(); in your constructor. – Bui Quang Huy Apr 08 '16 at 07:41
1

should be `new DatabaseHelper(context.getApplicationContext())` because context may be an activity that later will not exist any more. The ApplicationContext survives Activity live cycle – k3b Apr 08 '16 at 09:23
I think after create the instance of DatabaseHelper the context will belong to databasehelper and it not relate to Activity anymore – Bui Quang Huy Apr 08 '16 at 09:26
activity is the context, it inherits from context. So DatabaseHelper may internally reference an activity that may have been destoryed – k3b Apr 08 '16 at 09:31
@BuiQuangHuy This is slightly worse than OPs version IMO. First because the naming convention - you should use `sInstance` for `static` references, and second because it makes more sense to use the application context, as @k3b explained. – PPartisan Apr 08 '16 at 10:04

Is this a correct way to do singleton for SqliteHelper in android?

1 Answers1