Yahtzee game (using arrays and object classes)

Question

As part of an assignment for my intro Java class (please bear with my beginner skill set), I'm working on a program for which a Die object class is used to play a Yahtzee game. An overview of the directions are as follows:

The program should give an introduction of what it is going to do, ask the user how many dice the he/she wants to roll (number must be positive), and create an array of Die objects of the specified size. The program should then repeatedly roll all the dice until a Yahtzee is rolled occurs when all the dice have the same face value and report how many rolls it took to obtain the Yahtzee, and what face value made the Yahtzee. The program should give the user the option to run it again.

Also, it should print out a single period each time the dice are rolled, so as to indicate to the user that the program is working. These periods will be printed on the same line and a newline character should be added after every 50 rolls. The use of helper methods is encouraged and most methods should be around 12 lines or less. There should be, at minimum, a method that accepts an array of Die objects as a parameter and rolls each Die object once and another method that accepts the same array as a parameter and tests to see if a Yahtzee has been rolled.

I have completed the program and it seems to work alright, but I was hoping to get some advice/suggestions on how to better optimize my code, as I feel it is a little messy. Any feedback would be much appreciated!

Below is the Die class:

Note: The methods' modifiers, return types, and parameters were defined in the assignment directions and must be exactly as specified. There must be a single instance variable int faceValue, a void method roll with no parameters, a method getValue that simply returns the value of faceValue, and a method toString that returns a String representation of faceValue.

public class Die {
    int faceValue;
    public void roll(){
        faceValue = (int)(Math.random() * 6 + 1);
    }

    public int getValue(){
        return faceValue;   
    }

    public String toString(){
        return Integer.toString(faceValue);
    }
}

The Die class is to be used in the main program called Yahtzee, which I have written as follows:

import java.util.*;


public class Yahtzee {

    public static void main(String[] args) {
        Scanner keyboard = new Scanner (System.in);
            introduction();
            boolean runAgain = true;
            while (runAgain){
                boolean rolledYahtzee = false;
                int numRolls = 0;
                Die [] dieArray = dieArray(keyboard);
                while(!rolledYahtzee){
                    dieArray = rollDice (dieArray);
                    numRolls++;
                    printDots(numRolls);
                    rolledYahtzee = checkForYahtzee(dieArray, numRolls);
                }
                printResults (numRolls, dieArray);
                runAgain = runAgain(keyboard);
            }
    }

    public static void introduction(){
        System.out.println("This program will");
    }


    public static Die [] dieArray(Scanner keyboard){
        int numDice = getInt(keyboard, "How many dice do you want to throw? ");
        while (numDice <= 0){
            System.out.println("Sorry, you must enter a positive number.");
            numDice = getInt(keyboard, "How many dice do you want to throw? ");
        }
        Die [] dieArray = new Die [numDice];
        return dieArray;
    }

    public static int getInt (Scanner keyboard, String prompt){
        System.out.print(prompt);
        while (!keyboard.hasNextInt()){
            keyboard.next();
            System.out.println("Sorry, you must enter an integer.");
            System.out.print(prompt);
        }
        return keyboard.nextInt();
    }

    public static Die [] rollDice(Die [] dieArray){
        for (int i = 0; i < dieArray.length; i++){
            dieArray[i] = new Die ();
            dieArray[i].roll();
            dieArray[i].faceValue = dieArray[i].getValue();
        }
        return dieArray;
    }

    public static void printDots(int numRolls){
        System.out.print(".");
        if ((numRolls%50) == 0){
            System.out.println();
        }
    }

    public static boolean checkForYahtzee(Die[] dieArray, int numRolls) {
        for(int i = 0; i < dieArray.length; i++) {
            for(int j = i+1; j < dieArray.length; j++) {
                if(dieArray[i].getValue() != dieArray[j].getValue()) {
                    return false;
                }
            }
        }
        return true;
    }

    public static void printResults (int numRolls, Die[] dieArray){
        System.out.println();
        System.out.println("Yahtzee!!");
        System.out.print("After " + numRolls + " rolls, I finally rolled ");
        System.out.println(dieArray.length + " " + dieArray[0].getValue() + "'s");
        System.out.println();   
    }

    public static boolean runAgain (Scanner keyboard){
        System.out.print("Do you want to run another experiment? (y|n)):    ");
        String answer = keyboard.next().trim().toLowerCase();
        keyboard.nextLine();
        System.out.println();
        return (answer.charAt(0) == 'y');
    }
}

Here is a sample execution given by the instructor to demonstrate how the program should behave:

This program will... [your job to fill in]
How many dice do you want to throw? -2
Sorry, you must enter a positive number.
How many dice do you want to throw? 0
Sorry, you must enter a positive number.
How many dice do you want to throw? 2
........
Yahtzee!!
After 8 rolls, I finally rolled 2 1's
Do you want to run another experiment? (y|n)): y
How many dice do you want to throw? 3
..
Yahtzee!!
After 2 rolls, I finally rolled 3 2's
Do you want to run another experiment? (y|n)): y
How many dice do you want to throw? 4
..................................................
..............................
Yahtzee!!
After 80 rolls, I finally rolled 4 6's
Do you want to run another experiment? (y|n)): y
How many dice do you want to throw? 5
..................................................
..................................................
..................................................
..................................................
..................................................
..................................................
..................................................
..................................................
................................................
Yahtzee!!
After 448 rolls, I finally rolled 5 4's
Do you want to run another experiment? (y|n)): y
How many dice do you want to throw? 6
..................................................
..................................................
..................................................
[a bunch of dots removed to save space]
..................................................
..................................................
..........................
Yahtzee!!
After 15926 rolls, I finally rolled 6 5's
Do you want to run another experiment? (y|n)): n

Answer 1

For a first effort, I think it is pretty good so far. I can think of a few suggestions, though they may be overkill for a project this small in scope.

Separate UI from Game Logic

To me being object oriented would be more than just a Die class. I would also suggest have a YahtzeeGameEngine class, that would handle all the logic for the game itself. This way with the Die class and engine class, you could re-use the same logic and have a completely different interface (let's say you go from a command-line output to a GUI, you code currently is heavily tied to outputting to a command-line interface - which is fine for a class project, but a couple of the cornerstones of OO programming is code re-use and separation of concerns). The engine class can set up the number of sides on a die, handle the rolling, return the number of rolls to get a yahtzee win, etc). You could even do a roll event, so that the interface could trigger a function each time a die was rolled to encapsulate the functionality even further (print a dot or play a roll dice animation for a GUI - an event trigger might be too advanced for this project scope). Then your main class could handle all of the UI aspects, asking if they want to play again, displaying the rolls, and getting the number of dice.

Naming Conventions and Single Purpose

Another suggestion would just be some naming conventions. "printDots" sounds like it is printing more than one at a time, "printDot" would be more appropriate. Another example is "dieArray", if I see a function named that, I am not going to assume it is requesting the number dice and creating the die array at same time. Typically functions should be small and have a single role/purpose. This makes debugging easier and usually leads to less bugs overall. I would suggest splitting this into two functions or limiting its role to just getting the value from the user. Perhaps call the function something like "getNumberOfDiceToRoll()", and then create the array in the calling function.

Overall I think you did pretty good :)

Answer 2

Before I criticize your code, I must say this is really a good piece of code and you're heading in a good direction. There is some really good stuff and all the following comments are just to make it better.

rollDice

Like @toto2 said, this method could be better. I'll add a little thing, you can use for-each when you loop over an array. I would argue about the utility of doing the new operation. When I roll a die, I don't need a new one every time I need to roll, so can't you just roll ? It will re-roll the die. So I would have done something like :

private static void rollDice(Die [] dieArray){
    for (Die die : dieArray){
        die.roll();
    }
}

The new was not necessary and roll is already assigned a the value to faceValue so the last line was not necessary dieArray[i].faceValue = dieArray[i].getValue();. I've change the visibility as well, but I'll talk about this later.

dieArray

Well first, I'm sorry but this is a bad name. It does not say what it will do, just what it will return. There is some duplication too (I've added a foreach loop to initialize the array since I've modified the other method) :

public static Die [] dieArray(Scanner keyboard){
    int numDice = getInt(keyboard, "How many dice do you want to throw? ");
    while (numDice <= 0){
        System.out.println("Sorry, you must enter a positive number.");
        numDice = getInt(keyboard, "How many dice do you want to throw? ");
    }
    Die [] dieArray = new Die [numDice];
    for(int i = 0; i < dieArray.length; i++) {
      dieArray[i] = new Die();
    }
    return dieArray;
}

The duplication is this line numDice = getInt(keyboard, "How many dice do you want to throw? ");. The problem is if you need to change the text, you have two place to change it and what if you don't want to use getInt but another method, then oops you need to change both. The reason is why do you have a duplication ? It's because getInt does not verify for the rule that the number must be >=0. I would suggest you have a method getNumberOfDice that will call getInt and would check if the input is positive.

Visibility

Every methods of your class are public. When you make something public it means you want everyone in the world (I'm exaggerating) to use your function. I'm not sure this is what you need. In fact the modifier I use by default is private and after that I will ask myself do I need to make this public. For a class like that, almost nothing need to be public.

Static

Every thing is static too, is it a requirement ? I do not like using static that easily for something that could be easily an instance. I think you could drop the staticness (it's a new word I made up) of your class, this could help you realize that almost every method need an dieArray that could be an instance variable (this would clean out a bunch of methods arguments).

---

One minor nitpick, Scanner.in is not the keyboard, it's the console that called the program (either your IDE or your OS console), I would not be that precise about this variable name, input could be enough.

---

I'll leave this part here even if this class should not be modified by the OP, it's important to understand that this class is not well designed (IMO)

Die

Your Die as a pattern I have some difficulty with, it's when you need to call a method before calling another one. I need to call roll before I call getValue. This can be troublesome since I need a special knowledge before calling your class. You have a couple of options to deal with this. In your case, I would probably return the value of faceValue in the roll method (In fact, I would expect roll to return something but that's just me).

public int roll(){
    faceValue = (int)(Math.random() * 6 + 1);
    return faceValue;
}

Small comment about naming: the get() method should contain the complete name of the variable of the get call, so getValue should be getFaceValue.

faceValue = (int)(Math.random() * 6 + 1)

I would argue that 6 is a magic number here. Not every die as 6 faces, so you could make a small constant variable in your class to extract that magic variable, like private static int MAXIMUM_NUMBERS_FACES = 6 or something like that.

Answer 3

I don't like

public static Die [] rollDice(Die [] dieArray){
    for (int i = 0; i < dieArray.length; i++){
        dieArray[i] = new Die ();
        dieArray[i].roll();
        dieArray[i].faceValue = dieArray[i].getValue();
    }
    return dieArray;
}

• You are both modifying dieArray and returning it. Either modify it and return nothing, or just take an argument which is a number of dice and return a new array.

• You are creating a new Die() for each element of the array. Either already have an array with existing dice, create a whole new array.

I also don't like checkForYahtzee. Simpler is:

int value0 = dieArray[0].getValue();
for(int i = 1; i < dieArray.length; i++)
   if (dieArray[i].getValue() != value0)
      return false;
return true;

My last comment is a bit more advanced, and is somewhat inappropriate for your assignment: I would not define a Die class at all. You really only need to generate lists (of some given length) of random integers between 1 and 6. I would just write a method that does this and it would be sufficient, not to mention that it would be more efficient. However, you should probably not do this on your homework since your instructor would probably take many points off and tell you that you really don't get object oriented programming.

Answer 4

Lots of people have already given great input, so I'll focus on one small aspect.

runAgain()

1. runAgain() is a poorly chosen name, since it isn't actually running again. Consider something like promptToRunAgain() or better yet shouldRunAgain(). In general, your function name should describe what it does or what it checks for.

2. Even if you don't want to change that name, it is bad practice to name a variable the same as a function. If that variable is used solely to hold the value of the function, consider whether it's necessary at all. Avoid stuff like runAgain = runAgain(); as it will only become more confusing if you choose to delve into languages with variable functions / function expressions.

3. runAgain() only ever makes sense in the context of a console. Since you probably won't create another Scanner instance, you could possibly move your Scanner to class-level. Then you'd be able to remove the parameter from runAgain():

   public class Yahtzee {
       private static Scanner keyboard = new Scanner(System.in);
       public static void main(String[] args) {
       [...]
       public static boolean runAgain() {
   

4. You seem to be using while loops when in fact do..while loops may be clearer. Consider changing the following:

   boolean runAgain = true;
   while (runAgain) {
       boolean rolledYahtzee = false;
       [...]
       while (!rolledYahtzee) {
           [...]
           rolledYahtzee = checkForYahtzee(dieArray, numRolls);
       }
       [...]
       runAgain = runAgain(keyboard);
   }
   

   to:

   do {
       [...]
       do {
           [...]
       } while (!checkForYahtzee(dieArray, numRolls))
       [...]
   } while (shouldRunAgain());
   

Answer 5

I'm a little hesitant about this message printed in dieArray()'s loop:

System.out.println("Sorry, you must enter a positive number.");

It seems more fitting to print something like this:

System.out.println("Sorry, you must throw at least one die.");

This makes it sound (at least to me) less like a mathematical program and more like a game.

In addition, you could consider setting an upper limit to the number of dice that can be thrown, in case the user (strangely) inputs a huge number (which I assume can also break other code).

You could then present the user with a message like this:

System.out.println("Sorry, you cannot throw that many dice.");