simple two players dice throwing game

Question

I'm a first year uni student. We were asked to come up with code that can represent a simple two players dice throwing game (specifications are given below).

I know as a matter of fact that there are many areas upon which my code could be improved. However, rather than asking other people to correct my code, I would much prefer to be given hints or general principles as to how my code could be improved. I think I could get more out of this exercise this way.

So could someone please provide me with some comments or suggestions? Critics of any level and kind are welcomed! Feel free to pick apart my code!

Game class:

import java.util.Scanner;

public class Game {

    

    private Player p1;
    private Player p2;
    private Dice dice;
    private int scoreToWin;

    void displayGameMenu() {
        System.out.println();
        System.out.println("(1) Start a new game");
        System.out.println("(2) Play one round");
        System.out.println("(3) Who is leading now?");
        System.out.println("(4) Display game help");
        System.out.println("(5) Exit game");
        System.out.print("Choose an option: ");
    }

    void selectGameOption(int optionSelected) {
        switch (optionSelected) {
            case 1:
                this.startNewGame();
                break;
            case 2:
                this.playOneRound(p1);
                this.playOneRound(p2);
                break;
            case 3:
                this.whoIsLeading();
                break;
            case 4:
                this.displayGameInstruction();
                break;
            default:
                break;
        }
    }

    void startNewGame() {
        String p1Name;
        String p2Name;

        Scanner sc = new Scanner(System.in);
        System.out.print("Please enter player one name: ");
        p1Name = sc.nextLine();
        System.out.print("Please enter player two name: ");
        p2Name = sc.nextLine();
        System.out.print("Please enter the maximum score required to win: ");
        scoreToWin = sc.nextInt();

        p1 = new Player(p1Name);
        p2 = new Player(p2Name);
        dice = new Dice();
    }

    void playOneRound(Player p) {
        int result;

        int firstDiceRoll = dice.rollDice();
        int secondDiceRoll = dice.rollDice();
        if (firstDiceRoll == secondDiceRoll) {
            result = (firstDiceRoll + secondDiceRoll) * 2;
            p.setTotalScore(result);
            System.out.printf("%s rolled %d and %d, "
                    + "and scored %d points(BONUS DOUBLE POINTS), "
                    + "for a total of %d points",
                    p.getName(), firstDiceRoll, secondDiceRoll,
                    result, p.getTotalScore()
            );
        } else {
            result = (firstDiceRoll + secondDiceRoll);
            p.setTotalScore(result);
            System.out.printf("%s rolled %d and %d, "
                    + "and scored %d points, "
                    + "for a total of %d points",
                    p.getName(), firstDiceRoll, secondDiceRoll,
                    result, p.getTotalScore()
            );
        }

        System.out.println();
    }

    void whoIsLeading() {
        if (p1.getTotalScore() == p2.getTotalScore()) {
            System.out.format("Its currently a draw, "
                    + "%s has %d, %s has %d",
                    p1.getName(), p1.getTotalScore(),
                    p2.getName(), p2.getTotalScore()
            );
        } else if (p1.getTotalScore() > p2.getTotalScore()) {
            System.out.printf("%s is leading, %s has %d points, "
                    + "%s has %d points",
                    p1.getName(), p1.getName(), p1.getTotalScore(),
                    p2.getName(), p2.getTotalScore());
        } else if (p1.getTotalScore() < p2.getTotalScore()) {
            System.out.format("%s is leading, %s has %d points, "
                    + "%s has %d points.",
                    p2.getName(), p2.getName(), p2.getTotalScore(),
                    p1.getName(), p1.getTotalScore()
            );
        }
    }

    void displayGameInstruction() {
        System.out.println();
        System.out.println("All players roll a dice twice per turn.");
        System.out.println("If 2 dice rolls have the same value, the player scores 2 times the sum two dice rolls.");
        System.out.println("If 2 dice rolls have different values, the player simply scores the sum of two dice rolls.");
        System.out.println("For each player, result is incremented after each turn.");
        System.out.println("First player to reach or exceed the maxScore wins the game");

    }

    boolean checkIfAnyoneHasWon() {
        if (p1.getTotalScore() >= scoreToWin && p2.getTotalScore() >= scoreToWin) {
            System.out.println("Its a draw! Both players have exceeded the score limit");
            return true;
        } else if (p1.getTotalScore() >= scoreToWin && p2.getTotalScore() < scoreToWin) {
            System.out.format("%s won", p1.getName());
            return true;
        } else if (p1.getTotalScore() < scoreToWin && p2.getTotalScore() >= scoreToWin) {
            System.out.format("%s won", p2.getName());
            return true;
        }
        return false;
    }

public static void main(String[] args) {
        System.out.println("Welcome to the Dice and Roll game!");

        Game game = new Game();

        int optionSelected;

        while (true) {
            game.displayGameMenu();
            System.out.println();
            Scanner sc = new Scanner(System.in);
            optionSelected = sc.nextInt();

            while (optionSelected > 5 || optionSelected < 0) {

                System.out.print("Option entered invalid, please enter a number from 1 to 5: ");
                optionSelected = sc.nextInt();
            }

            if (optionSelected == 5) {
                System.out.println("Exiting game");
                break;
            }

            game.selectGameOption(optionSelected);

            boolean anyoneWin = game.checkIfAnyoneHasWon();
            if (anyoneWin) {
                System.out.println();
                System.out.println("Game ended.");
                break;
            }
        }
    }

}

Player class:

public class Player {
    private final String name;
    private int totalScore;
    
    Player(String name){
        this.name = name;
    }
    
    String getName(){
        return name;
    }
    
    int getTotalScore(){
        return totalScore;
    }
    
    void setTotalScore(int score){
        totalScore += score;
    }

}

Dice class:

import java.util.Random;

public class Dice {

    private final static int numberOfSides = 6;

    int rollDice() {

        int result;
        Random randomNumberGenerator = new Random();
        result = randomNumberGenerator.nextInt(numberOfSides) + 1;
        return result;
    }
}

Program specification:

For this assignment you will write a program that simulates a rather simplistic Dice Throwing Game. This section specifies the required functionality of the program. Only a text interface is required for this program; however, more marks will be gained for a program that is easy/intuitive to use, with clear information/error messages to the user.

The aim of the Dice Throwing Game is to simulate a simple game for 2 players, where they take turn to each roll a dice twice, and score points according to the results of the dice rolls. The winner is the one who accumulates a pre-defined maximum score first. Your program will display a menu which allows the user of the program to select various options to simulate the various operations. Results of all the operations will be printed on the screen as plain text only (eg. “Andy rolled 5 + 3, and scored 8 points”).

The “dice rolls” are simulated by the program, using some random number generator. The program will update each player's current score accordingly. For this assignment, the program will only handle TWO players. It will keep track of the current score of the players until one, or both, reaches the pre-defined maximum score, agreed upon at the start of the game.

Program Logic:

The Dice Throwing Game begins with a welcome message followed by a menu with the following options :

Option (1) asks the 2 players to enter their names. A player’s name must not be blank (or consists of only spaces and nothing else), but may contain spaces between the characters. If this option is chosen again after the players have already been set up, 2 "new" players are set up (ie. with 2 new names, and both their starting scores set to 0). Note that the new players replace the previous players – there are only ever two players at any one time.

After the names are set up, the game asks for a maximum score. The default maximum score should be set to 200 points. Each player’s initial score is set to 0.

Option (2) simulates the “dice roll” operations for both players. When this option is chosen, the computer generates 4 random numbers between 1-6 (ie. simulating a 6-sided dice), representing 2 dice rolls for each player. It then updates both players' scores accordingly. The scoring rules for each "round" are as follows :

• if the 2 dice rolls have the same value (ie. 1&1, 2&2, …, 6x6), the player scores 2 times the sum of that value (eg. 1&1 scores 4 points, 2&2 scores 8 points, etc)

• if the 2 dice rolls have different values, the player simply scores the sum of that value (eg. 1&4 scores 5 points, 5&2 scores 7 points, etc)

• if both players reaches a score which is more than the pre-defined maximum, the game’s result is a Draw. note that both players can reach over that score at the same time, since for each round, 2 dice rolls are performed for each player, before a winner is decided

• a player is considered a winner if he accumulates a score which is more than the pre-defined maximum, and the other player has not reached that score

Option (3) shows the players current scores, including who is leading the game.

Option (4) displays some brief instructions regarding how to play the game.

Option (5) exits the whole program. All player statistics should be cleared.

Additional Notes :

The menu must be displayed repeatedly after each operation, until the user chooses Option (5). Inputs other than 1-5 should be rejected, with appropriate error messages printed.

If the user chooses Option (2)/(3), before a game has been set up (Option (1)), an appropriate error message should be printed, and the operation aborted.

Your program must deal with invalid values entered by the user in a sensible manner.

Answer 1

Awkward split of responsibilities

The displayGameMenu method prints a list of options. This method is called from the main method, which is also in charge of validating the input. There are some issues with this:

• The code of displayGameMenu and main are far away from each other: I have to scroll to see both. This is a problem because when I look at main it's not obvious what number 5 is. And when adding a new menu option, it's easily possible to forget to update one of the relevant locations.

• The main method should simply call other methods, and have no logic of its own. The main method is designed for execution, not really a functional part of a class. As such, it is awkward that the responsibility of handling the menu is split between a member method (displayGameMenu) and a static method (main)

I suggest to reduce main to something like this:

public static void main(String[] args) {
    new Game().run();
}

And I suggest to eliminate displayGameMenu, the code will be easier to understand if the menu options are easily visible next to the input validation code.

Limit the scope

In this code:

int optionSelected;

while (true) {
    // ...
    optionSelected = sc.nextInt();

The variable optionSelected is not needed outside of the loop. So it's better to declare it inside:

while (true) {
    // ...
    int optionSelected = sc.nextInt();

Similarly, in playOneRound, you don't need to declare the result variable at the top, it's better to declare it in each of the branches of if-else.

Declare variables right before you need them

In startNewGame you declare p1Name and p2Name at the top. That's unnecessary. It's better to declare them when you assign them, for example:

String p1Name = sc.nextLine();

It's ok to reuse Random

In Dice.rollDice, you recreate an instance of Random on every call. This is unnecessary. It would be more efficient to store a single instance in private final Random random and reuse it across calls to rollDice.

Naming

A name like setTotalScore(int score) sounds like a classic setter that sets the value of a field to the given value. But this method in Player doesn't set to this value, it appends this to the current value. So addScore would be more natural.

The convention for naming constants is SHOUT_CASE in Java. So numberOfSides in Dice should be named NUMBER_OF_SIDES. Actually I'd just call it simply SIDES.

The word "Dice" in the rollDice method of the Dice class is redundant: the name of the class already implies that you're rolling a dice, not your eyes.

Pointless local variables

The result variable in Dice.rollDice is really unnecessary and tedious. The method could be reduced to a single line:

int roll() {
    return random.nextInt(SIDES) + 1;
}

Order of conditions

Instead of this:

while (optionSelected > 5 || optionSelected < 0) {

It might be slightly more readable if the terms are in a consistent order:

while (optionSelected < 0 || 5 < optionSelected) {

Another alternative:

while (!(0 <= optionSelected && optionSelected <= 5)) {

Order of modifiers

In private final static int numberOfSides the modifiers are not in the conventional order. The recommended ordering is private static final.

Answer 2

Your program must deal with invalid values entered by the user in a sensible manner.

Your code will give you InputMismatchException for input 'banana' on the main menu. Of course, if random things like bananas are stuffed into a program, not a whole lot of good will come out, but I think it was intended that there would be an error message, not a crash.

Answer 3

Even though the code is short and pretty enough, I think you'd benefit from reading about the strategy pattern and state machines. Go ahead and google it for a wealth of resources on good programming techniques.
You'd also be much more confident in how it works and how to arrive at those patterns by employing unit-testing and test driven development. (JUnit is a good start)

Good luck with your assignment!

Answer 4

I realize this is old, but since I happened to look at it today...

@janos' answer says that it is more efficient to use a single Random rather than generating a new one each time. That's true, but another reason to use a single Random is that it is more random.

When you create a Random instance, by default, it seeds the random number generator with some value. A common seed is the current time. If your computer runs more quickly than the granularity of the time check, this can result in two consecutive "random" values starting the sequence from the same point. The result? The same value (sequence) is generated for both.

If you create one Random instance and use it throughout the program, you get the full randomness of the generator. The generator itself will give you the randomness. If you constantly reseed, you are relying on the seeding process to give you randomness. The reseeding process is something that you write, so it puts the burden on you to get it right. The random number generator is part of Java, so the burden is on the Java language developers to get it right. Unless you are an expert in random number generation yourself, you are almost certainly better off using the version from the Java language developers rather than rolling your own.

TL;DR: make one Random instance and use it throughout the program to maximize the randomness of your results.