2D board game: good Model part?

Question

First time writing a big project in OOP. I am quite used to scientific programming but not to OOP, and even less to building GUIs. I am writing a 2D board game: the player can move on a map from tile to tile, meet Helpers and Enemies, and win a Trophy in the end.

I used the MVC pattern and the Model parts consist of the following classes: Board, Tile, Player, Opponent (abstract), Enemy, Helper, Trophy, Position, and HighScoreManager.

Putting them all here would be too much I guess, so here are the "core" files for the Model part:

Board.java

package model;

import java.util.Observable;

public class Board extends Observable {

    // VARIABLES 

    private Player player;
    private Tile[][] grid = new Tile[WIDTH][HEIGHT];
    private Trophy trophy;
    private Position activePosition = initialPosition;
    private boolean gameFinished = false;
    private HighScoreManager highScoreManager = new HighScoreManager();
    // constants 
    static final int WIDTH = 10; // in number of tiles
    static final int HEIGHT = 10; // in number of tiles
    static final int TOTAL_NUMBER_OF_ENEMIES = 4; 
    static final int TOTAL_NUMBER_OF_HELPERS = 4;
    // user input
    int DIFFICULTY_LEVEL = 1;
    // initial values
    static final int initialXPosition = 0;
    static final int initialYPosition = 0;
    static final Position initialPosition = new Position(initialXPosition, initialYPosition);
    static final int xTrophy = (int) (WIDTH*0.75);
    static final int yTrophy = (int) (HEIGHT*0.75);

    // CONSTRUCTOR


    // METHODS 

    public void initBoard() {

        player = new Player(initialPosition, DIFFICULTY_LEVEL);
        trophy = new Trophy();
        activePosition = initialPosition;
        gameFinished = false;
        highScoreManager.setHighScoreValue();

        int numberOfHelpers = 0;
        int numberOfEnemies = 0;

        // create all the tiles
        for (int i=0; i<WIDTH; i++) {
            for (int j=0; j<HEIGHT; j++) {
                this.grid[i][j] = new Tile("grass");
            }
        }
        // add player
        grid[initialXPosition][initialYPosition].setPlayer(player);
        // add trophy
        grid[xTrophy][yTrophy].setOpponent(trophy); 
        // randomly add enemies (only on even coordinates to spread them)
        while (numberOfEnemies < TOTAL_NUMBER_OF_ENEMIES) {
            int x = 2*(1 + (int)(Math.random()*(WIDTH/2-1)));
            int y = 2*(1 + (int)(Math.random()*(HEIGHT/2-1)));
            if (grid[x][y].getOpponent() == null) {
                grid[x][y].setOpponent(new Enemy());
                numberOfEnemies ++;
            }
        }
        // randomly add helpers (only on even coordinates to spread them)
        while (numberOfHelpers < TOTAL_NUMBER_OF_HELPERS) {
            int x = 2*(1 + (int)(Math.random()*(WIDTH/2-1)));
            int y = 2*(1 + (int)(Math.random()*(HEIGHT/2-1)));
            if (grid[x][y].getOpponent() == null) {
                grid[x][y].setOpponent(new Helper());
                numberOfHelpers ++;
            }
        }
        this.setChanged();
        this.notifyObservers();
    }


    public boolean isGameOver() {
        return (player.getStepsLeft() == 0 && trophy.getWon());
    }

    public Position computeDestination(String direction) {
        Position position = player.getPosition();
        Position destination = new Position(-10,-10); // TODO CHANGE THAT
        switch(direction) {
        case "left":    
            destination = position.plus(0,-1);
            break;
        case "right":   
            destination = position.plus(0,+1);
            break;
        case "down":    
            destination = position.plus(+1,0);
            break;
        case "up":  
            destination = position.plus(-1,0);
            break;
        default: 
            break;
        }

        return destination;
    }

    public boolean isWithinBounds(Position destination) {
        int x = destination.getX();
        int y = destination.getY();
        return (    0 <= x 
                &&  x < WIDTH 
                &&  0 <= y 
                &&  y < HEIGHT);
    }

    public boolean isAdjacent(Position destination) {
        Position position = player.getPosition();
        return(     position.plus(0, +1).equals(destination)
                ||  position.plus(0, -1).equals(destination)
                ||  position.plus(+1, 0).equals(destination)
                ||  position.plus(-1, 0).equals(destination)); 
    }

    public boolean isValidDestination(Position destination) {
        return (isAdjacent(destination) && isWithinBounds(destination)) ;
    }

    public void makeMove(Position destination) {
        if (isValidDestination(destination)) {
            int oldX = player.getPosition().getX();
            int oldY = player.getPosition().getY();
            int newX = destination.getX();
            int newY = destination.getY();
            grid[oldX][oldY].setPlayer(null); // remove player from old tile
            player.move(destination); // move player
            grid[newX][newY].setPlayer(player); // set player on new tile
            this.setChanged(); 
            this.notifyObservers(); // notify view of position change
            activePosition = player.getPosition();
        }
    }

    public boolean isInteractionPossible() {
        int x = player.getPosition().getX();
        int y = player.getPosition().getY();
        return (grid[x][y].isInteractionPossible());
    }

    public void handleInteraction(Player player) {
        int x = player.getPosition().getX();
        int y = player.getPosition().getY();
        if (grid[x][y].isInteractionPossible()) {
            grid[x][y].handleInteraction(player);
        }
        this.setChanged();
        this.notifyObservers(player);
        if (this.getPlayer().getStepsLeft()==0 || trophy.getWon()) { // check if game is finished
            gameFinished = true;
            highScoreManager.setScore(this.getPlayer().getScore());
        }
    }

    // GETTERS

    public Player getPlayer() {
        return player;
    }

    public Tile[][] getGrid() {
        return grid;
    }

    public Tile getTile(int i, int j) {
        return grid[i][j];
    }

    public Trophy getTrophy() {
        return trophy;
    }

    public Position getActivePosition() {
        return activePosition;
    }

    public boolean getGameFinished() {
        return gameFinished;
    }

    public HighScoreManager getHighScoreManager() {
        return highScoreManager;
    }

    public Tile getActiveTile() {
        int x = activePosition.getX();
        int y = activePosition.getY();
        return grid[x][y];
    }

}

Player.java

    package model;

    public class Player {

        // VARIABLES 

        private Position position;
        private int score = 0;
        private int stepsLeft;
        private int fightingSkill;
        private int jokingSkill;
        private int visionScope = 2;
        private String skillChoice;

        // CONSTRUCTOR 

        public Player(Position position, int difficultyLevel) {
            this.position = position;

            switch(difficultyLevel) {
            case 1: 
                this.stepsLeft = 150; 
                this.fightingSkill = 5; 
                this.jokingSkill = 5;

            case 2: 
                this.stepsLeft = 150; 
                this.fightingSkill = 2; 
                this.jokingSkill = 2;

            case 3: 
                this.stepsLeft = 100; 
                this.fightingSkill = 2; 
                this.jokingSkill = 2;

            case 4: 
                this.stepsLeft = 10; 
                this.fightingSkill = 1; 
                this.jokingSkill = 1;
            }
        }

        // METHODS ------------------------------

        public void move(Position destination) {
            setPosition(destination);
            stepsLeft -= 1;
        }

        public void increaseScore(int amount) {
            score += amount;
        }

        public void increaseStepsLeft(int amount) {
            stepsLeft+= amount;
        }

        public void increaseFightingSkill(int amount) {
            fightingSkill += amount;
        }

        public void increaseJokingSkill(int amount) {
            jokingSkill += amount;
        }

        // GETTERS 

        public Position getPosition() {
            return position;
        }

        public int getScore() {
            return score;
        }

        public int getStepsLeft() {
            return stepsLeft;
        }

        public int getFightingSkill() {
            return fightingSkill;
        }

        public int getJokingSkill() {
            return jokingSkill;
        }

        public String getSkillChoice() {
            return skillChoice;
        }

        public int getVisionScope() {
            return visionScope;
        }

        // SETTERS

        public void setPosition(Position position) {
            this.position = position;
        }

        public void setSkillChoice(int choice) {
            switch(choice) {
            case 0:
                this.skillChoice = "joke";
                break;
            case 1:
                this.skillChoice = "fight";
                break;
            case 2:
                this.skillChoice = "steps";
                break;
            default:
                ;
            }
        }

        @Override
        public String toString() {
            return "play \t" + position + "\t"+ stepsLeft
                    + "\t"+ jokingSkill + "\t"+ score + "\n";
        }
    }

Opponent.java

    package model;

    public abstract class Opponent {

        // VARIABLES

        private int bonus; // amount of skill points the player earns from that opponent
        private static final int MAX_BONUS = 5;

        // CONSTRUCTOR

        public Opponent() {
            this.bonus = 1 + (int)(Math.random()*(MAX_BONUS-1)); // generate bonus between 1 and 5
        }

        // METHODS

        public abstract void interactWith(Player player);

        public void increaseFightingSkill(Player player) {
            player.increaseFightingSkill(bonus);
        }

        public void increaseJokingSkill(Player player) {
            player.increaseJokingSkill(bonus);
        }

        // GETTERS

        public int getBonus() {
            return bonus;
        }

        // SETTERS

        public void setBonus(int bonus) {
            this.bonus = bonus;
        }
    }

Enemy.java

package model;

public class Enemy extends Opponent{

    //  VARIABLES 
    private static final int MAX_JOKING_THRESHOLD = 10;
    private static final int MAX_FIGHTING_THRESHOLD = 10;
    private int jokeThreshold;
    private int fightThreshold;
    private String[] options = {"joke", "fight"};

    // CONSTRUCTOR

    public Enemy() {
        super();
        this.jokeThreshold = 1 + (int) (Math.random()*(MAX_JOKING_THRESHOLD - 1));
        this.fightThreshold = 1 + (int) (Math.random()*(MAX_FIGHTING_THRESHOLD - 1));
    }

    // METHODS

    public void interactWith(Player player) {
        String choice = player.getSkillChoice();
        if (choice=="fight" && player.getFightingSkill() > fightThreshold) {
            increaseFightingSkill(player);
            player.increaseScore(this.getBonus());
        } else if (choice=="joke" && player.getJokingSkill() > jokeThreshold) {
            this.increaseJokingSkill(player);
            player.increaseScore(this.getBonus());
        }
    }

    // GETTERS

    public String[] getOptions() {
        return options;
    }

    // SETTERS

    public String toString() {
        return "enem";
    }

}

Helper.java

package model;

public class Helper extends Opponent {

    /*
     * When the player encounters a Helper, the player is asked what skill he wants to
     * improve (or get extra steps), and gets extra points in the skill of his choice 
     */

    // VARIABLES

    private static final int MIN_STEPS_BONUS = 5;
    private int stepsBonus;
    private String[] options = {"joke", "fight", "steps"};

    // CONSTRUCTOR

    public Helper() {
        super();
        this.stepsBonus =  1 + (int)(3*(Math.random()*(MIN_STEPS_BONUS -1))); // generate magic between 5 and 15
    }

    // METHODS

    public void interactWith(Player player) {
        String choice = player.getSkillChoice();
        if (choice == "fight") {
            this.increaseFightingSkill(player);
        } else if (choice == "joke") {
            this.increaseJokingSkill(player);
        } else if (choice == "steps") {
            this.increaseStepsLeft(player);
        }
    }

    public void increaseStepsLeft(Player player) {
        player.increaseStepsLeft(stepsBonus);
    }

    // GETTERS

    public int getStepsBonus() {
        return stepsBonus;
    }

    public String[] getOptions() {
        return options;
    }

    // SETTERS

    public String toString() {
        return "help";
    }
}

Tile.java

package model;

public class Tile {

    // VARIABLES 

    private String terrain;
    private Opponent opponent;
    private Player player;

    // CONSTRUCTORS

    public Tile(String terrain) {
        this.terrain = terrain;
    }

    // METHODS 

    public boolean isInteractionPossible() {
        return (opponent != null);
    }

    public void handleInteraction(Player player) {
        this.opponent.interactWith(player);
    }

    // GETTERS 

    public String getTerrain() {
        return terrain;
    }

    public Opponent getOpponent() {
        return opponent;
    }

    public Player getPlayer() { 
        return player;
    }

    // SETTERS 

    public void setTerrain(String terrain) {
        this.terrain = terrain;
    }

    public void setOpponent(Opponent opponent) {
        this.opponent = opponent;
    }

    public void setPlayer(Player player) {
        this.player = player;
    }

    @Override 
    public String toString() {
        return "\n"+player + "\t"+opponent;
    }
}

My questions

1. Any comment about structure or good practice is welcome, there is probably plenty to say.

2. I tried using the Observer-Observable pattern making Board the only observable to make it "philosophically simple". But then I feel like it gets messy when I want to notify different events to the GUI. Is there a better structure or a better way to send the notifications?

3. I've tried as much as possible to keep each class in the model independent on its own as much as possible, to be able to reuse a HighScoreManager for an other game for example. I don't whether it is always the best choice.

In case you want to see it, the whole project is on GitHub.

Answer 1

Things that I liked

• Separation of Code and UI.
• Clear naming of methods and variables. I didn't need any documentation with it, the names were enough.

Things that I didn't like

• Why? Why would you use this everywhere? It is not required and reduces readibility.
• No need to label things like // VARIABLES, // CONSTRUCTOR. If you are using an IDE like Eclipse use Ctrl + O, you can see all the methods, constructors and what not.

Things that you should include in your project

• More description about the game in README.md.
• LICENSE, the code over here is CC-3.0 licensed, but you haven't mentioned any in your GitHub project.

Some Suggestions

1. Use of Negation

Use negation to reduce the indentation and increase readibility. (Though it might be a little confusing at first)

Old onKeyPressed()

if (!board.getGameFinished()) { // keep moving only is game not finished
    ...
    if (direction!="") {
        destination = board.computeDestination(direction);
        ...
    }
}

Changed onKeyPressed()

if (board.getGameFinished())
    return;
...
if (direction == "")
    return;
...
destination = board.computeDestination(direction);
board.makeMove(destination);

---

2. Use of Switch-Case statement

Avoid using MAGIC NUMBERS.

Usage of numbers like -1, 0, 1. What do these numbers mean? Maybe you can write comment and would be able to understand. But after 2-3 months you wouldn't be able to figure out their meaning.

switch (box.getChoice()) { // take action depending on user's answer to 
    case -1: //close
        break;
    case 0: //new game
        board.initBoard(); 
        break;
    case 1: //high scores
        String highScore = board.getHighScoreManager().getHighScore();
        gui.showHighScore(highScore);
        break;
    case 2: //close
        gui.askExitConfirmation();
        break;
    default:
        ;
}

Instead, make them private static final variables and use. Or better yet make an enum out of them.

public enum EndOfGame {
    CLOSE_1(-1), NEW_GAME(0), HIGH_SCORES(1), CLOSE_2(2);
    private int value;
    private EndOfGame(int value) {
        this.value = value;
    }
    public int getValue() {
        return value;
    }
    public static EndOfGame fromValue(int value) {
        for (EndOfGame endOfGame : EndOfGame.values()) {
            if (endOfGame.getValue() == value)
                return endOfGame;
        }
        return null;
    }
}

and use it for switch-case as -

switch (EndOfGame.fromValue(box.getChoice())) {
case CLOSE_1:
    break;
case NEW_GAME:
    board.initBoard();
    break;
case HIGH_SCORES:
    String highScore = board.getHighScoreManager().getHighScore();
    gui.showHighScore(highScore);
    break;
case CLOSE_2:
    gui.askExitConfirmation();
    break;
default:
    break;
}

---

3. Comments and Indentation

Ctrl + Shitf + F is your friend. Use it often, it will format your code for you.

Don't write code like this with comments describing every variable

public EndOfGameDialogBox(JPanel parentPane) {
    choice = JOptionPane.showOptionDialog(  parentPane, //parent pane
        message,
        title,
        JOptionPane.YES_NO_CANCEL_OPTION, //type of options
        JOptionPane.QUESTION_MESSAGE, //type of message
        null, //icon
        options, //list of buttons
        options[0]); //default focus on first button
}

Instead, attach javadoc to your project, and you will be able to see the description of all the method arguments.

---

4. UI code on UI Thread

Go through the description about running the code on EDT as described here.