5
\$\begingroup\$

I just started to learn Java and had to write a "guess the number" game.

Could you please review and give me some feedback?

public static void main(String[] args) {

    System.out.println("Welcome to the guess the number game!\n");
    System.out.println("Please specify the configuration of the game:\n");

    Scanner input = new Scanner(System.in);

    System.out.println("Range start number (inclusively):");
    int startRange;
    startRange = nextValidInt(input);

    System.out.println("Range end (inclusively):");
    int endRange;
    endRange = nextValidInt(input);

    System.out.println("Maximum number of attemps:");
    int maxAttemp;
    maxAttemp = nextValidInt(input);

    System.out.println("Your Task is to guess the random number between "
            + startRange + " and " + endRange);

    Random randGenerator = new Random();
    int randNumber = randGenerator.nextInt((endRange - startRange + 1)
            + startRange);
    int numberOfTries = 0;

    System.out
            .println("You may exit the game by typing; exit - you may now start to guess:");

    for (numberOfTries = 0; numberOfTries <= maxAttemp - 1; numberOfTries++) {

        int guess;
        guess = nextValidInt(input);

        if (guess == randNumber) {
            System.out.println("Congratz - you have made it!!");
            System.out.println("Goodbye");
            break;
        } else if (guess > randNumber) {
            System.out.println("The number is smaller");
        } else if (guess < randNumber) {
            System.out.println("The number is higher");
        }

    }

    if (numberOfTries >= maxAttemp) {
        System.out.println("You reached the max Number of attempts :-/");
    }

}

public static int nextValidInt(Scanner s) {
    while (!s.hasNextInt() && s.hasNext()) {
        System.out.println("Please provide a valid integer value!");
        s.next();
    }
    return s.nextInt();

}
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Welcome to CodeReview. This isn't really a place to ask for help implementing new functionality (I edited your question to be on-topic). But we are more than happy to review the code that you do have. \$\endgroup\$ Commented Oct 12, 2014 at 20:12
  • 1
    \$\begingroup\$ This would be a great project for learning more about classes. I would suggest writing a GuessingGame class that handles all the logic of the game (e.g. number of guesses remaining), while leaving all the input/output in main, then posting that for review. \$\endgroup\$ Commented Oct 13, 2014 at 4:45

3 Answers 3

Reset to default
6
\$\begingroup\$

Not a full review, just a couple of points that jumped out at me:

  • you often assign the value to a variable on the next line after declaring it. Just assign the value on the same line (eg int startRange = nextValidInt(input);).
  • you can declare a loop variable inside the loop: for (int numberOfTries = 0; numberOfTries <= maxAttemp - 1; numberOfTries++).
  • if (numberOfTries >= maxAttemp) will always be true, as the for loop increases numberOfTries to maxAttemp. Well, except when you won. I would remove the if and replace the break with a return.
  • numberOfTries <= maxAttemp - 1 looks better written as numberOfTries < maxAttemp.
  • the main method shouldn't do anything except start your program. I would move your code to its own function. You might also want to create extra functions for initial info printing/setup gathering and the actual guessing game.
\$\endgroup\$
6
\$\begingroup\$

Not so random Random bug

int randNumber = randGenerator.nextInt((endRange - startRange + 1)
        + startRange);

Let's say that startRange = 10, and endRange = 25, then this will evaluate to:

int randNumber = randGenerator.nextInt((25 - 10 + 1) + 10);

int randNumber = randGenerator.nextInt((16) + 10);

int randNumber = randGenerator.nextInt(26);

Which will produce numbers in the range 0 -- 25. But... startRange was set to 10 !?

That's because your parenthesis is incorrect. Correct would be:

int randNumber = randGenerator.nextInt(endRange - startRange + 1) + startRange;

Which evaluates, with the same example numbers, to:

int randNumber = randGenerator.nextInt(25 - 10 + 1) + 10;

int randNumber = randGenerator.nextInt(16) + 10;

So the lowest possible number is 0 + 10 and the highest 15 + 10, that looks about right!

A variable name

int maxAttemp;

I'm not quite sure why you left that variable name unfini

I'd recommend naming it maxAttempts. Cutting the last two characters helps no one.

If-else-if-else-if...else?

if (guess == randNumber) {
    ...
} else if (guess > randNumber) {
    ...
} else if (guess < randNumber) {
    ...
}

I can assure you, if a number is not equal to, and not more than another number, it will be less than.

if (guess == randNumber) {
    ...
} else if (guess > randNumber) {
    ...
} else {
    ...
}

Or, as I would prefer to write it: If it's not more, and not less, then it is equal.

if (guess > randNumber) {
    ...
} else if (guess < randNumber) {
    ...
} else {
    ...
}
\$\endgroup\$
4
\$\begingroup\$

By naming your function nextValidInt, you have limited yourself. The only input that you can ever get is an integer. If you wanted to get a string (e.g. "exit"), you'd have to refactor the function to name it something like nextValidToken. The code would need to change as well, since the return type would no longer work. 

public static void findNextValidToken(Scanner s) {
    while ( ( ! s.hasNextInt() && ! s.hasNext("exit") ) && s.hasNext()) {
        System.out.println("Please provide a valid integer value or type exit to quit!");
        s.next();
    }
}

In your for loop, I don't see any reason for the inconsistent variable names. Why tries and attempts? Why not 

for (int attemptCount = 0; attemptCount < attemptMaximum; attemptCount++) {
\$\endgroup\$
1
  • \$\begingroup\$ Could you give me a hint on how to do refactor the code, to make "exit" work? \$\endgroup\$ Commented Oct 14, 2014 at 13:50

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.