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 ;)