Guess how many are in the jar game in Java - Take 2

Original question can be found here

I took the advice I got to heart, and re-wrote it. Once again I'd appreciate any advice as to how I can improve this using best practices.

Jar.java

package com.tn.jar;

public class Jar {
    private String itemName;
    private int numberOfItems;
    private int numberToGuess;
    private int numberOfGuesses;

    /*
     *  Default constructor 
     */

    public Jar() {
        this.numberOfGuesses = 0;
    }

    /*
     *  Getters
     */

    public String getItemName() {
        return itemName;
    }

    public void setItemName(String itemName) {
        this.itemName = itemName;
    }

    public int getNumberOfItems() {
        return numberOfItems;
    }

    public void setNumberOfItems(int numberOfItems) {
        this.numberOfItems = numberOfItems;
    }

    public int getNumberToGuess() {
        return numberToGuess;
    }

    /*
     *  Setters
     */

    public void setNumberToGuess(int numberToGuess) {
        this.numberToGuess = numberToGuess;
    }

    public int getNumberOfGuesses() {
        return numberOfGuesses;
    }

    public void setNumberOfGuesses(int numberOfGuesses) {
        this.numberOfGuesses = numberOfGuesses;
    }

    public void incrementNumberOfGuesses() {
        numberOfGuesses++;
    }
}

Prompter.java

```
package com.tn.jar;

import java.util.Random;
import java.util.Scanner;

public class Prompter {
private Jar jar;
private Scanner scanner;

/*
* Public constructor
*/

public Prompter(Jar jar) {
this.jar = jar;
this.scanner = new Scanner(System.in);
}

/*
* Method that actually starts the game
*/

public void play() {
adminSetup();
playerSetup();
}

/*
* Admin setup
*/

private void adminSetup() {
printTitle("Administrator setup");
setupGame();
}

/*
* Player setup
*/

private void playerSetup() {
printTitle("Playe

Now this is a lot easier to follow than your previous version. Though, there are still a few things.

/*
 *  Getters
 */

If you require such comments to separate your classes into regions, you're doing something wrong.

Personally, I like to have the following layout and order to my class:

• Constants

• Static members

• Members

• Static constructor

• Constructor

• Static methods

• Methods

• Subclasses

And within all these categories everything is sorted alphabetically. Now many people will frown when I say "sort your stuff alphabetically", because many prefer to sort it logically, which means that the first used functions are first. I prefer alphabetically because there is never a doubt where a function is and you can easily navigate the whole file (you're seeing a function named "measure" and you know that "getValues" is above it and "setName" is below it).

But that is my personal preference.

/*
 *  Utility method for checking if the string read by the system can be parsed as an integer
 */

private static boolean isNan(String string) {

You obviously misunderstood what Javadoc is. It is not only commenting above the functions, it is associating documentation with a function. Well formed Javadoc would look this:

/**
 * Checks if the given {@link String} is not a number.
 * 
 * The given {@link String} is considered to be not a number if
 * the {@link Integer#parseInt} function fails on it.
 *
 * @param string The string to check.
 * @return {@code true} if the given value is not a number.
 * @see Integer#parseInt
 */
private static boolean isNan(String string) {

Note that tow asterisks which indicate that this is not a comment, but a Javadoc block. From this countless tools and IDEs are able to associate the Javadoc block with the function below and it automatically provide and generate documentation for this function.

Prompter is not only prompting, it is doing everything. I therefor suggest that you rename Game to Main, and Prompter to Game.

Even though your naming is great, it is a little bit off from time to time. For exmaple:

• adminSetup does not setup something that has to do with administration tasks, it initializes the game.

• playerSetup does not only setup the player, it also starts the game.

• getGuess is not only getting a guess, it loops until the guess is correct.

Functions should be named after what they do, and they should only do that one thing.

String numberOfItems    = askQuestion("Maximum of lentils in the jar: ");;

while(isNan(numberOfItems)) {
    numberOfItems = askQuestion("Maximum of lentils in the jar: ");
}

jar.setItemName(itemName);
jar.setNumberOfItems(Integer.parseInt(numberOfItems));

Your askQuestion function does read and return a String, which you then try to parse as int and then make an int. It would be a lot better if you have separate functions, like this:

private String promptForString(String prompt);
private int promptForInt(String prompot);

Which would shorten your code to:

jar.setItemName(promptForString("Name of items in the jar: ");
jar.setNumberOfItems(promptForInt("Maximum of lentils in the jar: ");

The Scanner class does have multiple different read* methods, have a look at and use them.

String itemName         = askQuestion("Name of items in the jar: ");            
String numberOfItems    = askQuestion("Maximum of lentils in the jar: ");;

Again, my personal preference: I hate that aligning of statements, I have a hard time reading such code because my brain switches into some sort of "column mode", and I have a hard time associating the values with the names. But that might just be me.

Also, it is a lot of work to maintain.

Otherwise this feels like an improvement to me. It feels like a lot less code which is easier to follow.

StackExchange Code Review Q#134357, answer score: 2