Playing "craps" for the win

Question

No specific question here. I am beginning with Java and here is an exercise. The rules of game:

You roll two dice. Each die has six faces, which contain one, two, three, four, five and six spots, respectively. After the dice have come to rest, the sum of the spots on the two upward faces is calculated. If the sum is 7 or 11 on the first throw, you win. If the sum is 2, 3 or 12 on the first throw (called “craps”), you lose (i.e., the “house” wins). If the sum is 4, 5, 6, 8, 9 or 10 on the first throw, that sum becomes your “point.” To win, you must continue rolling the dice until you “make your point” (i.e., roll that same point value). You lose by rolling a 7 before making your point.

import java.io.IOException;
import java.util.Random;
import java.util.Scanner;



public class helloworld {
    private static int point;
    private static enum possibleResults {UNDEFINED, WIN, LOSE};
    private static possibleResults gameResult = possibleResults.UNDEFINED;
    private static String causeOfLose;
    private static String causeOfWin;

    public static int rollDice(int n) {
        int sum = 0;
        Random randomNumbers = new Random();
        for(int i=0; i<n; i++) {
            sum += (1 + randomNumbers.nextInt(6));
        }
        System.out.printf("Rolling dice... You got %d\n", sum);
        return sum;
    }

    public static void firstRoll() throws IOException {
        waitUser();
        int tmp = rollDice(2);
        if(tmp == 7 || tmp == 11) {
            gameResult = possibleResults.WIN;
            causeOfWin = String.format("you have got a lucky number in the first round: %d.", tmp);
        } else if(tmp == 2 || tmp == 3 || tmp == 12) {
            gameResult = possibleResults.LOSE;
            causeOfLose = String.format("you have got an unlucky number in the first round: %d.", tmp);
        } else {
            point = tmp;
            System.out.printf("Your point is %d, you need to make your point to win.%n", point);
        }
        declareResult();
    }

    public static void declareResult() {
        if(gameResult == possibleResults.LOSE) {
            System.out.printf("You have lost, because %s%n", causeOfLose);
        } else if(gameResult == possibleResults.WIN) {
            System.out.printf("You have won, because %s%n", causeOfWin);
        } else {
            System.out.println("Game continues...");
        }
    }

    public static void moreRolls() {
        Scanner inputScan = new Scanner(System.in);
        while(gameResult == possibleResults.UNDEFINED) {
            waitUser();
            int tmp = rollDice(2);
            if(tmp == point) {
                gameResult = possibleResults.WIN;
                causeOfWin = "you have made your point!";
            } else if(tmp == 7) {
                gameResult = possibleResults.LOSE;
                causeOfLose = "you have hit 7 before making your point.";
            } else {
                ;
            }
            declareResult();
        }
    }

    private static void waitUser() {
        Scanner inputScan = new Scanner(System.in);
        System.out.print("Press enter to start rolling: ");
        String input = inputScan.nextLine();
    }

    public static void main(String[] args) throws IOException {
        firstRoll();
        moreRolls();
    }

}

Any corrections and/or suggestions are welcome.

Answer 1

A class called helloworld? ClassNames in Java should have CapitalizedCamelCase, so your class should be HelloWorld, but Craps is probably a better name.

All your variables are static?

Creating a new Random instance each time you roll the dice is an unnecessary overhead. This is a case where you can have a static variable (it is thread-safe).

You call rollDice(2), and never any other input value. Why not make the method simply:

public static int rollDice() {
    return 2 + randomNumbers.nextInt(6) + randomNumbers.nextInt(6);
}

Note that methods should do one thing, and one thing only. Your rollDice was rolling the dice, and also printing the result. Printing the output should be the responsibility of some other method.

A switch statement will help your turn methods:

int roll = rollDice();
switch(roll) {
    case 7:
    case 11:
        gameResult = possibleResults.WIN;
        causeOfWin = String.format("you have got a lucky number in the first round: %d.", roll);
        break;
    case 2:
    case 3:
    case 12:
        gameResult = possibleResults.LOSE;
        causeOfLose = String.format("you have got an unlucky number in the first round: %d.", roll);
        break;
    default:
        point = roll;
        System.out.printf("Your point is %d, you need to make your point to win.%n", point);
}

Answer 2

• A code currently in main should be wrapped in its own method (game perhaps).

• Business logic shall be performed as high in the call tree as possible. Regarding your code this principle means that neither firstRoll nor moreRolls should call declareResults. The same (questionably) applies to the while loop in moreRolls.

  Consider a game skeleton along the lines of

  result = firstRoll();
  while (result == UNDEFINED)
      result = moreRoll();
  declareResults();
  

• moreRolls creates a Scanner and never uses it.

• causeOfWin and causeOfLose are mutually dependent. You don't need two distinct variables: cause would be enough.