Guessing game in Java - Take 3

Question

I already have two versions of this code reviewed (thanks @Bobby). They can be found here and here.

The question is still the same.

The goal is maintainability and following best practices.

Jar.java

package com.tn.jar;
import java.util.Random;
public class Jar {
 private String itemName;
 private int numberOfItems;
 private int numberToGuess;
 private int numberOfGuesses;
 public Jar() {
 this.itemName = "Default Name";
 this.numberOfItems = new Random().nextInt(10) + 1;
 this.numberToGuess = new Random().nextInt(this.numberOfItems) + 1;
 this.numberOfGuesses = 0;
 }
 public Jar(String itemName, int numberOfItems) {
 this.itemName = itemName;
 this.numberOfItems = numberOfItems;
 this.numberToGuess = new Random().nextInt(numberOfItems) + 1;
 this.numberOfGuesses = 0;
 }
 public String getItemName() {
 return itemName;
 }
 public int getNumberOfItems() {
 return numberOfItems;
 }
 public int getNumberToGuess() {
 return numberToGuess;
 }
 public int getNumberOfGuesses() {
 return numberOfGuesses;
 }
 public void incrementNumberOfGuesses() {
 numberOfGuesses++;
 }
}

Player.java

package com.tn.jar;
public interface Player {
 void playGameAsPlayer();
}

Admin.java

package com.tn.jar;
public interface Admin {
 void playGameAsAdmin();
}

Game.java

/*
 * 
 */
package com.tn.jar;
import java.util.InputMismatchException;
import java.util.Random;
import java.util.Scanner;
/**
 * The Class Game.
 */
public class Game implements Admin, Player {
 /** The jar. */
 Jar jar;
 /* (non-Javadoc)
 * @see com.tn.jar.Player#playGameAsPlayer()
 */
 @Override
 public void playGameAsPlayer() {
 Prompter.printTitle("Player");
 jar = new Jar();
 startGame();
 }
 /* (non-Javadoc)
 * @see com.tn.jar.Admin#playGameAsAdmin()
 */
 @Override
 public void playGameAsAdmin() {
 Prompter.printTitle("Administrator Setup");
 String itemName = Prompter.promptForString("Name of items in the jar: "); 
 int numberOfItems = Prompter.promptForInt("Maximum of lentils in the jar: ");
 jar = new Jar(itemName, numberOfItems); 
 startGame();
 };
 /**
 * Start game.
 */
 private void startGame() {
 printGameExplanation();
 Prompter.areYouReady();
 startGuessingLoop();
 printResult();
 }
 /**
 * Start guess loop.
 * Here we accept input from user, 
 * and keeps looping until the answer is correct
 */
 private void startGuessingLoop() {
 do {
 jar.incrementNumberOfGuesses();
 } while(Prompter.promptForInt("\nGuess: ") != jar.getNumberToGuess());
 }
 /**
 * Game explanation.
 */
 private void printGameExplanation() {
 System.out.printf("%nYour goal is to guess how many lentils are in the jar. Your guess should be between 1 and %d%n%n", 
 jar.getNumberOfItems());
 }
 /**
 * Prints the result.
 */
 private void printResult() {
 System.out.printf("%nCongratulations - you guessed that there are %d" + 
 " lentils in the jar! It took you %d" + 
 " guess(es) to get it right.%n", jar.getNumberToGuess(), jar.getNumberOfGuesses());
 }
 /**
 * The Class Prompter.
 */
 static class Prompter {
 /** The scanner. */
 private static Scanner scanner = new Scanner(System.in);
 /**
 * Are you ready.
 */
 public static void areYouReady() {
 do {
 System.out.print("Ready? (press ENTER to start guessing): "); 
 } while (scanner.nextLine().length( ) > 0);
 }
 /**
 * Prompt for input.
 *
 * @param question the question
 * @return the string
 */
 public static String promptForString(String question) {
 System.out.print(question);
 String result = scanner.nextLine();
 return result;
 }
 /**
 * Prompt for int.
 *
 * @param question The question you want to ask
 * @return result as an int
 */
 public static int promptForInt(String question) {
 System.out.print(question);
 int result = 0;
 boolean success = false;
 while(!success) {
 try {
 result = scanner.nextInt();
 success = true;
 } catch(InputMismatchException e) {
 System.out.print(question);
 scanner.nextLine();
 }
 }
 return result;
 }
 /**
 * Prints the title.
 *
 * @param title the title
 */
 public static void printTitle(String title) {
 System.out.printf("%n%s%n=========================%n%n", title.toUpperCase());
 }
 }
 /**
 * The main method.
 *
 * @param args the arguments
 */
 public static void main(String[] args) {
 new Game().playGameAsAdmin();
 }
}

Answer 1

Overall your code looks okay, nothing much to comment on it as a whole. I'd like to add the following comments though:

1. The variables in your Jar class should be final where possible, I would change them to the following:

   private final String itemName;
   private final int numberOfItems;
   private final int numberToGuess;
   private int numberOfGuesses = 0;
   

   Note that this also initializes numberOfGuesses to zero here instead of in the constructor.

2. Use a single constructor and call the other constructor to avoid code duplication:

   public Jar() {
    this("Default Name", new Random().nextInt(10) + 1);
   }
   public Jar(String itemName, int numberOfItems) {
    this.itemName = itemName;
    this.numberOfItems = numberOfItems;
    this.numberToGuess = new Random().nextInt(numberOfItems) + 1;
    this.numberOfGuesses = 0;
   }
   

   Also normally I would create all random numbers with one Random instance such that you could set a single seed in your program if you want to observe the same behavior. In this case this would be less easy but could still be achieved with a variable like private final Random random = new Random();.

3. You should deal with as less static variables as possible, meaning that your Prompter static class really should be a Prompter instance and that the Scanner variable should be local to that instance, ideally you should be able to pass along a scanner by for example doing Prompter prompter = new Prompter(new Scanner(System.in));.

4. In the Prompter#areYouReady method you could use while (!scanner.nextLine().isEmpty()) as loop condition.

5. In the Prompter#promptForInt method you could break; out of the loop, with the following code:

    public static int promptForInt(String question) {
    System.out.print(question);
    int result = 0;
    while (true) {
    try {
    result = scanner.nextInt();
    break;
    } catch (InputMismatchException e) {
    System.out.print(question);
    scanner.nextLine();
    }
    }
    return result;
    }
   

6. You could remove the dependency on System.out in your Prompter class by adding a dependency on a PrintStream, now your Prompter instance creation could look like: Prompter prompter = new Prompter(new Scanner(System.in), System.out);.

7. While your styling is in no place bad, it still has room for improvement. You inconsistently sometimes have two lines of white-space between methods, this should be only one. Your while statement lacks breathing space, it should be of the form while (condition) { }, not while(condition) { } and in one case you have unnecessary spacing in a method call, see the scanner.nextLine().length( ) call.

• Nilzone-: Great tips, Thank you! I was just wondering - how can I know when it's appropriate to use a static nested class over a nested class? I thought a static would be fine here since I really don't need an instance of it to call it's methods.
• skiwi: @Nilzone- I would suggest to use a top-level class over a nested static class in this case, it's just a matter of extensibility and testability to me, static classes and methods are very hard to test if they rely on static state as well.
• Simon Forsberg: @Nilzone- The static variables are really bad (in this case, your Scanner), static methods and static inner classes is perfectly alright in my opinion. The problem in your case is that nearly all of your static methods depends on the static variable, which makes them as well bad.