Dice throwing simulation in Java

Question

For one of my labs I need to write a complete java program which will simulate a "very simple" dice betting game. The specifications are as follows: simulate the throw of 3 dice. If you throw three sixes then you win $20; if you throw three of any other value then you win $10; if you throw two dice which are the same value then you win $5. If none of the conditions above are met, then you would lose $1.

Example Runs: Dice Throw

Dice 1 : 2     Dice 2 : 1    Dice 3 : 2
Congratulations : You threw TWO - 2s
You win $5

Dice 1 : 2    Dice 2 : 2    Dice 3 : 2
Congratulations : You threw THREE - 2s
You win $10

Dice 1 : 4    Dice 2 : 6    Dice 3 : 3
Unfortunately : You did not throw anything of value
You lose $1

The resulting code that I wrote to solve this problem is as follows:

/**
 * SRN: 507-147-9
 */
public class Lab6_part3 {

    public static void main(String[] args) {

        // define vars
        int round = 1;
        int dice1, dice2, dice3;

        while (round <= 3) {

            dice1 = 1 + (int)(Math.random() * 6);
            dice2 = 1 + (int)(Math.random() * 6);
            dice3 = 1 + (int)(Math.random() * 6);

            System.out.println();
            System.out.println("Dice 1 : " + dice1 + "    Dice 2 : " + dice2 + "    Dice 3 : " + dice3);

            // Three of a kind
            if ((dice1 == dice2) && (dice1 == dice3)) { // 3-of-a-kind (D1)                              

                // Rolls three sixes
                if (dice1 == 6) {
                    System.out.println("Congratulations : You threw THREE - 6s");
                    System.out.println("You win $20");
                } else {
                    // Rolls three of anything else
                    System.out.println("Congratulations : You threw THREE - " + dice1 + "s");
                    System.out.println("You win $10");
                }
            }
            
            // Two of a kind (PRINT "dice1")
            else if (dice1 == dice2 || dice1 == dice3) {
                System.out.println("Congratulations : You threw TWO - " + dice1 + "s");
                System.out.println("You win $5");
            } 
            
            // Two of a kind (PRINT "dice2")
            else if (dice2 == dice1 || dice2 == dice3) {                              
                System.out.println("Congratulations : You threw TWO - " + dice2 + "s");
                System.out.println("You win $5");
            }

            // Two of a kind (PRINT "dice3")
            else if (dice3 == dice1 || dice3 == dice2) {                              
                System.out.println("Congratulations : You threw TWO - " + dice3 + "s");
                System.out.println("You win $5");
            } 
            // Did not throw anything of value
            else {
                System.out.println("Unfortunately : You did not throw anything of value");
                System.out.println("You lose $1");
            }
            round++;
        }
    }
}

The problem that I'm running into with this approach is that although the code functions as it's supposed to, I would like to have a simpler way to write the two-of-a-kind instead of having three "if" statements. My goal in this, is to create a three-way "or" statement instead of comparing dice1 with dice 2 and 3, and dice 2 with dice 1 and 3, etc...

else if (dice1 == dice2 || dice1 == dice3 || dice2 == dice3) {                              
    System.out.println("Congratulations : You threw TWO - " + somethings?? + "s");
    System.out.println("You win $5"); 

If I were to do that, how would I then be able to print out the value of the pair which I've identified?

Answer 1

Welcome to CodeReview. Regarding your issue, a quick solution is:

else if (dice1 == dice2 || dice1 == dice3 || dice2 == dice3) {       
    int doubleNum = dice1 == dice2 ? dice1 : dice1 == dice3 ? dice1 : dice3;
    System.out.println("Congratulations : You threw TWO - " + doubleNum + "s");
    System.out.println("You win $5"); 
}

The ternary operator makes the code more compact, but in this case I think it's a bit less readable than a chain of if-else.

Few other suggestions:

• The while-loop can be replaced by a more convenient for-loop. From:

  int round = 1;
  while(round<=3){
      //...
      round++;
  }
  

  To:

  for(int round = 1; round <= 3; round++) {
      //...
  }
  

• Declaring multiple variables in one line int dice1, dice2, dice3; is not considered good practice in Java.
• Instead of generating a random float and then casting it to an int, generate directly an int with ThreadLocalRandom.current().nextInt(6). Docs.
• You can print the first line with System.out.format. From:

  System.out.println();
  System.out.println("Dice 1 : " + dice1 + "    Dice 2 : " + dice2 + "    Dice 3 : " + dice3);
  

  To:

  System.out.format("%nDice 1 :  %d    Dice 2 : %d    Dice 3 : %n", dice1, dice2, dice3)
  

• Class names should be PascalCase. Instead of Lab6_part3 you can name it Lab6Part3.
• Declare a constant for the number of rounds, to make it easier to change. For example:

  public class Lab6Part3 {
  
      private static final int ROUNDS = 3;
  
      public static void main(String[] args) {
          for(int round = 1; round <= ROUNDS; round++) {
              //...
          }
  }
  

Another approach is to generate three numbers with Random#ints and calculate the frequencies:

Random r = new Random();
// Generate three random numbers from 1 to 6
IntStream diceRolls = r.ints(3, 1, 7);

// Generate map of frequencies
Map<Integer, Long> freq = diceRolls.boxed()
        .collect(Collectors.groupingBy(Function.identity(), Collectors.counting()));

boolean winner = false;
for (Entry<Integer, Long> entry : freq.entrySet()) {
    int number = entry.getKey();
    long times = entry.getValue();
    if (times == 3) {
        // Three of a kind ...
        winner = true;
    } else if (times == 2) {
        // Two of a kind...
        System.out.println("Congratulations : You threw TWO - " + number + "s");
        System.out.println("You win $5");
        winner = true;
    }
}
if(!winner) {
    // Did not throw anything of value
}

Answer 2

You don't need to have 3 else_if statements. There are 3 scenarios where you are in a "two-fo-a-kind" case. dice1==dice2, dice1==dice3, dice2==dice3. Your first if block catches 2 of these scenarios. The only other option is if dice2==dice3.

If you think about it, your third else_If block would always create a true evaluation in one of the first two blocks, so your code will never reach it.

// Two of a kind (PRINT "dice1")
else if (dice1 == dice2 || dice1 == dice3) {
    System.out.println("Congratulations : You threw TWO - " + dice1 + "s");
    System.out.println("You win $5");
} 
        
// Two of a kind (PRINT "dice2")
else if (dice2 == dice3) {                              
    System.out.println("Congratulations : You threw TWO - " + dice2 + "s");
    System.out.println("You win $5");
}

Answer 3

        }
        
        // Two of a kind (PRINT "dice1")
        else if (dice1 == dice2 || dice1 == dice3) {

Please don't do this. If you want to use the half-cuddled else, please write it always on two consecutive lines.

        }
        else if (dice1 == dice2 || dice1 == dice3) {
          // Two of a kind (PRINT "dice1")

There are two reasons.

1. If I want to know that the structure is finished, this way I can see just by looking one line past the }. With your original, I would have to look an arbitrary number of lines to the next statement. This gets especially bad as the comment blocks get longer. It is entirely possible to write a comment that is taller than a single screen. Meaning that instead of simply scrolling a little farther, I would need to page down past the comment to see that there is an else block.
2. If someone uses something that reformats the code to put the else on the same line as the }, it will have to move the comment anyway. Then even if they format it back, the comment will move. This causes confusion in source control. So just put it the way that the reformatter would from the beginning.

C-style languages in general have confusing block handling. Because they use } both to end the structure and just to end the block in a continuing structure. I.e. they have no way to say that the if structure is ending versus continuing with an else of some sort. To compensate, we have to use coding conventions. And one of the simplest conventions, which transcends style, is to never put a curly-brace (} or {) more than one line away from the associated keyword if there is one.