SQLite database for books

Question

First, my Book object:

public class Book {

    public static final String TABLE_NAME = "BOOK";
    public static final String COLUMN_ID = "ID";
    public static final String COLUMN_TITLE = "TITLE";

    private long id;
    private String title;

    public Book(String title) {
        this.id = -1;
        this.title = title;
    }

    public Book(long id, String title) {
        this.id = id;
        this.title = title;
    }

    public long getId() {
        return id;
    }

    public void setId(long id) {
        this.id = id;
    }

    public String getTitle() {
        return this.title;
    }

    public void setTitle(String title) {
        this.title = title;
    }
}

My Database helper:

public class DatabaseHelper extends SQLiteOpenHelper {
    private volatile SQLiteDatabase mDatabase;
    private Context mContext;

    private static final String DB_NAME = "data.db";
    private static final int DB_VERSION = 1;

    public DatabaseHelper(Context context) {
        super(context.getApplicationContext(), DB_NAME, null, DB_VERSION);
        mContext = context.getApplicationContext();
    }

    private synchronized void open() throws SQLException {
        mDatabase = getWritableDatabase();
    }

    @Override
    public void onCreate(SQLiteDatabase db) {
        DatabaseSetup.createTables(db);
    }

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

    }

    //********CRUD OPERATIONS************

    public synchronized long createBook(Book book) {
        open(); //opens DB
        ContentValues values = new ContentValues();
        values.put(Book.COLUMN_TITLE, book.getTitle());
        long id = mDatabase.insertWithOnConflict(Book.TABLE_NAME, null, values, SQLiteDatabase.CONFLICT_IGNORE);
        close(); //closes DB
        return id;
    }

    public synchronized Book getBookById(long id) {
        open(); //opens DB
        Cursor cursor = mDatabase.query(
                Book.TABLE_NAME, // table
                new String[]{Book.COLUMN_ID, Book.COLUMN_TITLE}, // column names
                Book.COLUMN_ID + " = ?", // where clause
                new String[]{id + ""}, // where params
                null, // groupby
                null, // having
                null);  // orderby

        Book book = null;

        if (cursor != null && cursor.getCount() > 0) {
            cursor.moveToFirst();
            String title = getStringFromColumnName(cursor, Book.COLUMN_TITLE);
            book = new Book(id, title);
            cursor.close();
        }
        close(); //closes DB
        return book;
    }

    public synchronized long updateBook(Book book) {
        open(); //opens DB
        long bookId = book.getId();
        ContentValues values = new ContentValues();
        values.put(Book.COLUMN_TITLE, book.getTitle());
        long numUpdated = mDatabase.updateWithOnConflict(
                Book.TABLE_NAME, // table
                values, // values
                Book.COLUMN_ID + " = ?", // where clause
                new String[]{bookId + ""}, // where params
                SQLiteDatabase.CONFLICT_IGNORE);
        close(); //closes DB
        return numUpdated;
    }

    public synchronized void deleteBook(Book book) {
        open(); //opens DB
        long bookId = book.getId();
        mDatabase.delete(
                Book.TABLE_NAME, // table
                Book.COLUMN_ID + " = ?", // where clause
                new String[]{bookId + ""}); // where params
        close(); //closes DB
    }

    //********HELPER FUNCTIONS************

    public static String getStringFromColumnName(Cursor cursor, String columnName) {
        int columnIndex = cursor.getColumnIndex(columnName);
        return cursor.getString(columnIndex);
    }
}

And my Database setup class which handles create-table queries

public class DatabaseSetup {

    //create-table queries

    public static final String CREATE_TABLE_BOOK_STRING =
            "CREATE TABLE " + Book.TABLE_NAME + " ("
                    + Book.COLUMN_ID + " INTEGER PRIMARY KEY AUTOINCREMENT, "
                    + Book.COLUMN_TITLE + " TEXT NOT NULL)";

    public static void createTables(SQLiteDatabase db) {
        db.execSQL(CREATE_TABLE_BOOK_STRING);
    }
}

1. Is it considered a good idea to open and close the database in every CRUD function like I am doing? I do it this way because even if I were to just open the database in, for example, an onResume() call in an Activity, there is no guarantee the connection will still be open by the time I invoke a CRUD operation elsewhere in the Activity.

2. Do I need to be doing anything else to verify that the database is open / safe to perform operations on?

3. Is it even necessary to call close()?

4. Is it bad practice for me to be putting the table name / field names in the Book class?

5. Should I be putting the CRUD operations in their own separate class, e.g. public class BookCrudHandler?

6. Should I be putting the id in the Book class (so that I have some relationship between the object and the row in the database), or should I have some other kind of data structure that maps POJOs to their corresponding IDs in the database that I reference as a lookup?

7. Anything else / any comments / any practices I am missing that I should be using?

Answer 1

A review for each point, more like my choice than a standard ;)

1.Is it considered a good idea to open and close the database in every CRUD function like I am doing? I do it this way because even if I were to just open the database in, for example, an onResume() call in an Activity, there is no guarantee the connection will still be open by the time I invoke a CRUD operation elsewhere in the Activity.

It is the best to prevent any mistake, of course if you are going to call n times a CRUD method, this will cost some time so you could create the CRUD to take a varargs parameter `(Book... books) to be able to pass one or many values to a method. Then this is use like an Array so you loop on each to to the job (mainly for insert, update and delete). Using a PreparedStatement for these will reduce the compile time of the query.

2.Do I need to be doing anything else to verify that the database is open / safe to perform operations on?

If you close it correctly, you are the only one to have the access. So unless you are doint multithreading, this should not crash.

3.Is it even necessary to call close()?

I think you will end up with a locked database if you don't close it correctly. I had this error not so long ago because of this I think. This is probably an open door to memory leak too.

4.Is it bad practice for me to be putting the table name / field names in the Book class?

Since you didn't create a DAO class Data Access Object for the books to store these constants and the CRUD methods to manage the books. This is not so bad but the DAO is the best choice. If you never use DAO, you will find some guide everywhere.

5.Should I be putting the CRUD operations in their own separate class, e.g. public class BookCrudHandler?

Your handler is the DAO a was talking about. Regrouping the methods for a class or a group of class. In those, you can just call the methods insert ann not insertBook.

6.Should I be putting the id in the Book class (so that I have some relationship between the object and the row in the database), or should I have some other kind of data structure that maps POJOs to their corresponding IDs in the database that I reference as a lookup?

Both are correct but Maps cost memory. I usually add the id in the instance if I know that I will need it later (to recover more data for a specific instance).

7.Anything else / any comments / any practices I am missing that I should be using?

Well, I have my way, everybody has is way. Hard to answer this one ;)