Playing “craps” for the win v0.2 Continued from Playing &quot;craps&quot; for the win


  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.


// Craps.java

package co.vu.xxx.craps;

import java.io.IOException;
import java.security.SecureRandom;
import java.util.Scanner;

public class Craps {
    private int point;
    public static enum possibleResults {UNDEFINED, WIN, LOSE};
    protected possibleResults gameResult;
    private String cause;
    private static SecureRandom randomNumbers = new SecureRandom();

    public Craps() {
        gameResult = possibleResults.UNDEFINED;
    }

    public static int rollDice() {
        int randN = 2 + randomNumbers.nextInt(6) + randomNumbers.nextInt(6);
        System.out.printf("Rolling dice... You got %d%n", randN);
        return randN;
    }

    public void firstRoll() throws IOException {
        waitUser();
        int tmp = rollDice();
        switch (tmp) {
            case 7:case  11:
                gameResult = possibleResults.WIN;
                cause= String.format("you have got a lucky number in the first round: %d.", tmp);
                break;
            case 2:case 3:case 12:
                gameResult = possibleResults.LOSE;
                cause= String.format("you have got an unlucky number in the first round: %d.", tmp);
                break;
            default:
                point = tmp;
        }
    }

    public void declareResult() {
        if(point != 0) {
            System.out.printf("You need to make your point: %d%n", point);
        }
        switch (gameResult) {
            case LOSE:
                System.out.printf("You have lost, because %s%n", cause);
                break;
            case WIN:
                System.out.printf("You have won, because %s%n", cause);
                break;
            case UNDEFINED:
                System.out.println("Game continues...\n");
                break;
        }
        System.out.println();
    }

    public void moreRolls() {
        waitUser();
        int tmp = rollDice();
        if(tmp == point) {
            gameResult = possibleResults.WIN;
            cause = "you have made your point!";
        } else if(tmp == 7) {
            gameResult = possibleResults.LOSE;
            cause = "you have hit 7 before making your point.";
        }
    }

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




// UseCraps.java

package co.vu.xxx.craps;

import java.io.IOException;

public class UseCraps {
    public static void main(String[] args) throws IOException {
        Craps crapObj = new Craps();
        crapObj.firstRoll();
        crapObj.declareResult();
        while(crapObj.gameResult == Craps.possibleResults.UNDEFINED) {
            crapObj.moreRolls();
            crapObj.declareResult();
        }
    }
}
At a glance, the code looks reasonably good.

Either initialize everything in the constructor:

protected possibleResults gameResult;
private static SecureRandom randomNumbers;

public Craps() {
    this.gameResult = possibleResults.UNDEFINED;
    this.randomNumbers = new SecureRandom();
}


… or get rid of the constructor completely (it is implicit):

protected possibleResults gameResult = possibleResults.UNDEFINED;
private static SecureRandom randomNumbers = new SecureRandom();




There is a curious mix of private and protected members.  Why?  I suggest making them all private unless you see a need to expose them.

The gameResult member should be made private; other classes should have to call a public method getResult() to retrieve it.  That prevents other code from setting the gameResult.



As an enum, possibleResults should follow the naming conventions for classes (beginning with a capital letter).  I suggest shortening the name to

public static enum Outcome { UNDEFINED, WIN, LOSE };


Putting it as the first thing in the Craps class would be good, so as to avoid interrupting the class definition with an enum definition.