Listview from SQLite

Question

I'm trying to get back into programming by building an app I've had in mind for quite a while. I've created an SQLite database and have managed to get some data in it. I'm trying to display the data inside a Listview, and whilst it is displaying, I'm concerned I've made things more difficult for myself for making the Listview display the data in a more coherent fashion.

For reference, this is the tutorial I followed for creating my database: http://hmkcode.com/android-simple-sqlite-database-tutorial/

My question basically boils down to this: Have I done this the easiest way so far? If not, what do I need to do to fix it?

I'm also not entirely sure I'm populating the listview correctly in the Games class as I'm doing it in onCreate. I plan on having the ListView refresh when I insert a new row.

This is my SQLiteHelper class:

package bassios.initiativetracker;

import android.content.ContentValues;
import android.content.Context;
import android.database.Cursor;
import android.database.sqlite.SQLiteDatabase;
import android.database.sqlite.SQLiteOpenHelper;
import android.util.Log;

import java.util.LinkedList;
import java.util.List;

import bassios.initiativetracker.model.Game;
import bassios.initiativetracker.model.Player;

public class SQLiteHelper extends SQLiteOpenHelper {

private static final int DATABASE_VERSION = 1;
private static final String DATABASE_NAME = "InitiativeTracker";


public SQLiteHelper(Context context) {
    super(context, DATABASE_NAME, null, DATABASE_VERSION);
}

@Override
public void onCreate(SQLiteDatabase db) {
    String CREATE_PLAYER_TABLE = "CREATE TABLE Player (" +
            "playerId INTEGER PRIMARY KEY AUTOINCREMENT, " +
            "playerName TEXT, " +
            "characterName TEXT, " +
            "gameId INTEGER )";

    String CREATE_GAME_TABLE = "CREATE TABLE Game (" +
            "gameId INTEGER PRIMARY KEY AUTOINCREMENT, " +
            "gameName TEXT, " +
            "gameSystem TEXT )";

    db.execSQL(CREATE_PLAYER_TABLE);
    db.execSQL(CREATE_GAME_TABLE);
}

@Override
public void onUpgrade(SQLiteDatabase db, int oldVersion, int newVersion) {
    db.execSQL("DROP TABLE IF EXISTS Player, Game");

    this.onCreate(db);
}

/**
 * CRUD stuff follows
 */

//Table Names
private static final String TABLE_PLAYER = "Player";
private static final String TABLE_GAME = "Game";

//Table Columns
//Player Table
private static final String PLAYER_KEY_ID = "playerId";
private static final String PLAYER_KEY_PNAME = "playerName";
private static final String PLAYER_KEY_CNAME = "characterName";
private static final String PLAYER_KEY_GAMEID = "gameId";

//Game Table
private static final String GAME_KEY_ID = "gameId";
private static final String GAME_KEY_NAME = "gameName";
private static final String GAME_KEY_SYSTEM = "gameSystem";

//Columns Array
private static final String[] PLAYER_COLUMNS = {PLAYER_KEY_ID, PLAYER_KEY_PNAME, PLAYER_KEY_CNAME, PLAYER_KEY_GAMEID};
private static final String[] GAME_COLUMNS = {GAME_KEY_ID, GAME_KEY_NAME, GAME_KEY_SYSTEM};


public void addGame(Game game) {
    Log.d("addGame", game.toString());

    SQLiteDatabase db = this.getWritableDatabase();

    ContentValues values = new ContentValues();
    values.put(GAME_KEY_NAME, game.getGameName());
    values.put(GAME_KEY_SYSTEM, game.getGameSystem());

    db.insert(TABLE_GAME,
            null,
            values);

    db.close();
}

public void addPlayer(Player player) {
    Log.d("addPlayer", player.toString());

    SQLiteDatabase db = this.getWritableDatabase();

    ContentValues values = new ContentValues();
    values.put(PLAYER_KEY_PNAME, player.getPlayerName());
    values.put(PLAYER_KEY_CNAME, player.getCharacterName());
    values.put(PLAYER_KEY_GAMEID, player.getGameId());

    db.insert(TABLE_PLAYER,
            null,
            values);

    db.close();
}


public Game getGame(int id) {
    SQLiteDatabase db = this.getReadableDatabase();

    Cursor cursor = db.query(TABLE_GAME,
            GAME_COLUMNS,
            "gameId = ? ",
            new String[]{String.valueOf(id)},
            null,
            null,
            null,
            null);

    if (cursor != null) {
        cursor.moveToFirst();
    }

    Game game = new Game();
    game.setGameId(Integer.parseInt(cursor.getString(0)));
    game.setGameName(cursor.getString(1));
    game.setGameSystem(cursor.getString(2));

    Log.d("getGame(" + id + ")", game.toString());

    return game;
}

public Player getPlayer(int id) {
    SQLiteDatabase db = this.getReadableDatabase();

    Cursor cursor = db.query(TABLE_PLAYER,
            PLAYER_COLUMNS,
            "playerId = ? ",
            new String[]{String.valueOf(id)},
            null,
            null,
            null,
            null);

    if (cursor != null) {
        cursor.moveToFirst();
    }

    Player player = new Player();
    player.setPlayerId(Integer.parseInt(cursor.getString(0)));
    player.setPlayerName(cursor.getString(1));
    player.setCharacterName(cursor.getString(2));
    player.setGameId(Integer.parseInt(cursor.getString(3)));


    Log.d("getPlayer(" + id + ")", player.toString());

    return player;
}


public List<Game> getAllGames() {
    List<Game> games = new LinkedList<>();

    String query = "SELECT * FROM " + TABLE_GAME;
    SQLiteDatabase db = getWritableDatabase();
    Cursor cursor = db.rawQuery(query, null);

    Game game;
    if (cursor.moveToFirst()) {
        do {
            game = new Game();
            //game.setGameId(Integer.parseInt(cursor.getString(0)));
            game.setGameName(cursor.getString(1));
            game.setGameSystem(cursor.getString(2));

            games.add(game);
        } while (cursor.moveToNext());
    }
    Log.d("getAllGames()", games.toString());
    return games;
}

public List<Player> getAllPlayers() {
    List<Player> players = new LinkedList<>();

    String query = "SELECT * FROM " + TABLE_PLAYER;
    SQLiteDatabase db = getWritableDatabase();
    Cursor cursor = db.rawQuery(query, null);

    Player player = null;
    if (cursor.moveToFirst()) {
        do {
            player = new Player();
            player.setPlayerId(Integer.parseInt(cursor.getString(0)));
            player.setPlayerName(cursor.getString(1));
            player.setCharacterName(cursor.getString(2));
            player.setGameId(Integer.parseInt(cursor.getString(3)));

            players.add(player);
        } while (cursor.moveToNext());
    }
    Log.d("getAllPlayers()", players.toString());
    return players;
}


public int updateGame(Game game) {
    SQLiteDatabase db = getWritableDatabase();

    ContentValues values = new ContentValues();
    values.put("gameName", game.getGameName());
    values.put("gameSystem", game.getGameSystem());

    int i = db.update(TABLE_GAME, values, GAME_KEY_ID + " = ?", new String[]{String.valueOf(game.getGameId())});

    db.close();

    return i;
}

public int updatePlayer(Player player) {
    SQLiteDatabase db = getWritableDatabase();

    ContentValues values = new ContentValues();
    values.put("playerName", player.getPlayerName());
    values.put("characterName", player.getCharacterName());
    values.put("gameId", player.getGameId());

    int i = db.update(TABLE_PLAYER, values, PLAYER_KEY_ID + " = ?", new String[]{String.valueOf(player.getPlayerId())});

    db.close();

    return i;
}


public void deleteGame(Game game) {
    SQLiteDatabase db = getWritableDatabase();

    db.delete(TABLE_GAME, GAME_KEY_ID + " = ?", new String[]{String.valueOf(game.getGameId())});
    db.close();
    Log.d("deleteGame ", game.toString());
}

public void deletePlayer(Player player) {
    SQLiteDatabase db = getWritableDatabase();

    db.delete(TABLE_PLAYER, PLAYER_KEY_ID + " = ?", new String[]{String.valueOf(player.getPlayerId())});
    db.close();
    Log.d("deletePlayer ", player.toString());
  }
}

Game POJO class:

package bassios.initiativetracker.model;

public class Game {
private int gameId;
private String gameName;
private String gameSystem;

public Game() {
}

public Game(String gameName, String gameSystem) {
    super();
    this.gameName = gameName;
    this.gameSystem = gameSystem;
}

public int getGameId(){
    return gameId;
}

public void setGameId(int gameId){
    this.gameId = gameId;
}

public String getGameName() {
    return gameName;
}

public String getGameSystem() {
    return gameSystem;
}

public void setGameName(String gameName) {
    this.gameName = gameName;
}

public void setGameSystem(String gameSystem) {
    this.gameSystem = gameSystem;
}

@Override
public String toString() {
    return  gameName + ": " + gameSystem;
}
}

Games class:

package bassios.initiativetracker;

import android.app.AlertDialog;
import android.content.DialogInterface;
import android.os.Bundle;
import android.support.design.widget.FloatingActionButton;
import android.support.design.widget.Snackbar;
import android.support.v7.app.AppCompatActivity;
import android.support.v7.widget.Toolbar;
import android.view.LayoutInflater;
import android.view.View;
import android.widget.ArrayAdapter;
import android.widget.Button;
import android.widget.EditText;
import android.widget.ListView;
import java.util.List;
import bassios.initiativetracker.model.Game;

public class Games extends AppCompatActivity {
EditText gameName;
EditText gameSystem;

@Override
protected void onCreate(Bundle savedInstanceState) {
    super.onCreate(savedInstanceState);
    setContentView(R.layout.activity_game);


    Toolbar toolbar = (Toolbar) findViewById(R.id.toolbar);
    setSupportActionBar(toolbar);
    ListView listContent = (ListView) findViewById(R.id.gamesListView);
    FloatingActionButton fab = (FloatingActionButton) findViewById(R.id.fab);

    fab.setOnClickListener(new View.OnClickListener() {
        @Override
        public void onClick(View view) {
            showAddGameDialog();
        }
    });
    SQLiteHelper db = new SQLiteHelper(this);

    List<Game> game;
    game = db.getAllGames();
    ArrayAdapter adapter = new ArrayAdapter(this, android.R.layout.simple_expandable_list_item_1, game);
    listContent.setAdapter(adapter);

    getSupportActionBar().setDisplayHomeAsUpEnabled(true);
}

public void saveGame(String gameName, String gameSystem) {
    SQLiteHelper db = new SQLiteHelper(this);
    db.addGame(new Game(gameName, gameSystem));
}

public void showAddGameDialog() {
    AlertDialog.Builder dialogBuilder = new AlertDialog.Builder(this);
    LayoutInflater inflater = this.getLayoutInflater();

    final View dialogView = inflater.inflate(R.layout.save_game_dialog, null);


    dialogBuilder.setView(dialogView)
            .setTitle("Create Game")
            .setMessage("Enter game details")
            .setPositiveButton(getResources().getString(R.string.game_save), null)
            .setNegativeButton(getResources().getString(R.string.cancel), null);
    final AlertDialog b = dialogBuilder.create();

    b.setOnShowListener(new DialogInterface.OnShowListener() {
        @Override
        public void onShow(DialogInterface dialog) {
            Button saveGame = b.getButton(AlertDialog.BUTTON_POSITIVE);
            saveGame.setOnClickListener(new View.OnClickListener() {
                @Override
                public void onClick(View view) {
                    String gameName = ((EditText) dialogView.findViewById(R.id.gameNameField)).getText().toString();
                    String gameSystem = ((EditText) dialogView.findViewById(R.id.gameSystemField)).getText().toString();
                    if (gameName.isEmpty()) {
                        Snackbar.make(view, "Please enter a game name", Snackbar.LENGTH_LONG).setAction("Action", null).show();
                    } else {
                        if (gameSystem.isEmpty()) {
                            Snackbar.make(view, "Please enter a game system", Snackbar.LENGTH_LONG).setAction("Action", null).show();
                        } else {
                            saveGame(gameName, gameSystem);
                            b.dismiss();
                        }
                    }
                }
            });
        }
    });
    b.show();
}

}

Answer 1

One way you can improve this is by writing a data source class that does all your database access, instead of putting it inside the SQLiteOpenHelper. Leave that class to just creating and updating the database. Your data source could be like this:

public class GameDataSource {
   private SQLiteHelper mOpenHelper;
   private String[] gameColumns = new String[] {}; // Not required, but could be helpful to have.
   private SQLiteDatabase mDatabase;

   public GameDataSource(Context context) {
      mOpenHelper = new SQLiteHelper(context);
   }

   public void open() throws SQLiteException {
      mDatabase = mOpenHelper.getWriteableDatabase();
   }

   public void close() {
      mDatabase.close();
   }

   public void addGame(Game g) {
      // build values and insert
      long newId = mDatabase.insert(GAME_TABLE, null, values);
   }
}

Then you can access it from your activity. What I would do is create the datasource in onCreate, open it in onResume, and close it in onPause. All of this explained in this great tutorial that I reference a lot.

Keeping a single reference to the OpenHelper like this (it is known as a singleton) is helpful for preventing memory leaks. Something brief that I touched on in a blog post I wrote.

EDIT

Some more I might add is to create a contract class to keep track of your database schema, like this:

public class GameContract {
    public static class GameEntry implements BaseColumns{
        public static final String TABLE_NAME = "gameTable";
        public static final String COLUMN_NAME = "gameName";
        public static final String COLUMN_SYSTEM = "gameSystem";
    }

    public static class PlayerEntry implements BaseColumns {
        public static final String TABLE_NAME = "playerTable";
        public static final String COLUMN_NAME = "playerName";
        public static final String COLUMN_CHARACTER_NAME = "playerCharacterName";
        public static final String COLUMN_GAME = "playerGameId";
    }
}

That way you have string constants for names (which you already do though, great job on that) consolidated in one appropriate class (as I said earlier, leave the openhelper to updating/creating). You can still build your tables with it. Notice that I implemented BaseColumns which has an _ID string you can use as your primary key:

db.execSql(
   "CREATE TABLE " + GameEntry.TABLE_NAME + " (" + 
   GameEntry._ID + " INTEGER PRIMARY KEY, " +
   GameEntry.COLUMN_NAME + "TEXT NOT NULL, " +
   GameEntry.COLUMN_SYSTEM + "TEXT NOT NULL);"
}

And, now that you have that, I personally like to build my content values inside the object itself so I know where to find it:

public class Game {
    public ContentValues getContentValues() {
        ContentValues values = new ContentValues();

        if(getIdentifier() > 0) {
            values.put(GameEntry._ID, getIdentifier());
        }

        values.put(GameEntry.COLUMN_NAME, getName());
        values.put(GameEntry.COLUMN_SYSTEM, getSystem());

        return values;
    }
}

The edits are merely suggestions, they may or may not be considered best practice but they are things I do to stay organized and have worked really well for me so far.

EDIT 2

For displaying the information in the ListView, you can use a CursorAdapter. Here is a sample of what it could look like for you:

public class GameAdapter extends CursorAdapter {
    // Game fields
    public static final String[] GAME_COLUMNS = new String[] {
        GameEntry.TABLE_NAME + "." + GameEntry._ID,
        GameEntry.COLUMN_NAME,
        GameEntry.COLUMN_SYSTEM
    };

    // Indexes for columns to provide ease of access in `bindCursor` further down.
    // ID would be index 0, unused in my example.
    private static final int NAME_INDEX = 1;
    private static final int SYSTEM_INDEX = 2;

    public GameAdapter(Context context) {
        super(context, null, 0);
    }

    // Must override newView. 
    // Assumes a list item resource called list_item_game
    @Override
    public View newView(Context context, Cursor cusor, ViewGroup parent) {
        View view = LayoutInflater.from(context).inflatE(R.layout.list_item_game, parent, false);
        view.setTag(new GameViewHolder(view));
        return view;
    }

    // Must override bindView
    // Calls bindCursor inside the GameViewHolder
    @Override
    public void bindView(View view, Context context, Cursor cursor) {
        GameViewHolder viewHolder = (GameViewHolder) view.getTag();
        viewHolder.bindCursor(cursor);
    }

    // ViewHolder model allows for better scrolling.
    private static class GameViewHolder {
        // Just for example, consider TextViews for each field
        private final TextView nameView;
        private final TextView systemView;

        public ViewHolder(View view) {
            nameView = (TextView) view.findViewById(R.id.list_item_game_name);
            systemView = (TextView) view.findViewById(R.id.list_item_game_system);
        }

        // bindCursor takes in a cursor and sets the values of the data at this line
        public void bindCursor(Cursor cursor) {
            String name = cursor.getString(NAME_INDEX);
            String system = cursor.getString(SYSTEM_INDEX);

            nameView.setText(name);
            systemView.setText(system);
        }
    }
}

If you are using a ContentProvider, you can easily load the CursorAdapter and watch for any changes to the Game URI and it will update automatically if you use a CursorLoader. Will edit again if that is an option, otherwise simply get a cursor of all of your Game values and call swapCursor on the adapter:

// Sample code, query may differ
Cursor cursor = mDatasource.getAllGames();
mGameAdapter.swapCursor(cursor);

Then, assuming you've called mListView.setAdapter(mGameAdapter);, each time you call swapCursor on it you will see the updated games list.

I hope this helps you. If you'd like to learn more about SQLite in Android without the UI aspects, I also have a four post blog segment that will give you pretty broad coverage. NOTE these posts are geared toward a ContentProvider, perhaps I will do another segment on data access without one, but I think you've got the required knowledge down for that.