Guessing Game In Java

Question

import java.io.IOException;
import java.util.Scanner;

public class Main {

    private static int random = (int) (Math.random() * 100 + 1);
    private static int data;
    private static int go;
    private static int attempt = 10;

    public static void Start() throws IOException {

        Scanner scanInput = new Scanner(System.in);
        data = scanInput.nextInt();

        if (data > 100) {
            System.out.println("Only Enter Numbers Between 0-100");
            Start();
        } else if (data == random) {
            System.out.println("Wow, you guessed it!!");
            scanInput.close();
            System.exit(0);
        } else if (data < random) {
            go += 1;
            if (go == attempt) {
                System.out.println("You have guessed the wrong number " + go + " times, sorry but you'll have to try again! :(");
                scanInput.close();
                System.exit(0);
            }
            System.out.println("Higher");
            Start();
        } else if (data > random) {
            go += 1;
            if (go == attempt) {
                System.out.println("You have guessed the wrong number " + go + " times, sorry but you'll have to try again! :(");
                scanInput.close();
                System.exit(0);
            }
            System.out.println("Lower");
            Start();
        }
    }

    public static void main(String[] args) throws IOException {
        System.out.println("Guess A Number Between 0-100 Within " + attempt + " Goes");
        Start();
    }

}

Answer 1

I will try to point out things that can be improved in your code:

1. Removing complexity by using return:

    if (data > 100) {
        System.out.println("Only Enter Numbers Between 0-100");
        Start();
        return;
    }

    if (data == random) {
        System.out.println("Wow, you guessed it!!");
        scanInput.close();
        return;
    }

    if (data < random) {
        go ++;
        if (go == attempt) {
            System.out.println("You have guessed the wrong number " + go + " times, sorry but you'll have to try again! :(");
            scanInput.close();
            System.exit(0);
            return;
        }
        System.out.println("Higher");
        Start();
        return;
    }

    if (data > random) {
       // ...
    }

2. Try to avoid recursion, if it is not needed - your code can be easily converted not to use recursion:

public static void Start() throws IOException {

while (true) {

    if (go == attempt) {
        System.out.println("You have guessed the wrong number " + go + " times, sorry but you'll have to try again! :(");
        scanInput.close();
        System.exit(0);
        return;
    } 

    Scanner scanInput = new Scanner(System.in);
    data = scanInput.nextInt();

    if (data > 100) {
        System.out.println("Only Enter Numbers Between 0-100");
        continue;
    }

    if (data == random) {
        System.out.println("Wow, you guessed it!!");
        scanInput.close();
        break;
    } 

    if (data < random) {
        go ++;
        if (go == attempt) {
            System.out.println("You have guessed the wrong number " + go + " times, sorry but you'll have to try again! :(");
        continue;
    }
}

3. return value instead of System.exit(0);

In order to make your function more reusable, you can have it return a boolean specifying if the number was guessed or not. The caller can decide what to do with the information (maybe this should be integrated in some guessing Web service and those exits are not ok).

Also, it would make sense to allow the user to abort guessing.

// true if guesses, otherwise false
public static boolean Start( throws IOException {
    go = 0;

    while (true) {
        if (go == attempt) {
            System.out.println("You have guessed the wrong number " + go + " times, sorry but you'll have to try again! :(");
            scanInput.close();
            return false;
        }

        Scanner scanInput = new Scanner(System.in);
        data = scanInput.nextInt();

        // define an abort constant and inform the user what value abort the guessing process
        if (data == AbortNumber) {
            System.out.println("Guessing aborted by user");
            return false;
        }

        if (data > 100) {
            System.out.println("Only Enter Numbers Between 0-100");
            continue;
        }

        if (data == random) {
            System.out.println("Wow, you guessed it!!");
            scanInput.close();
            break;
        } 

        if (data < random) {
            go ++;
            if (go == attempt) {
                System.out.println("You have guessed the wrong number " + go + " times, sorry but you'll have to try again! :(");
            continue;
        }
}

4. Variable initializations - go is initialized with 0, but it is better to put it explicitly. It is particularly useful, when your guessing instance is reused, as go is changed during the guessing process.

5. Proper names for variables - it is recommended to provide meaningful names for your variables. E.g.:

data -> userGuess
go -> step
attempt -> maxAttempts

6. Avoid repetition - use a constant for maximum number range and use it across your code:

const int MaxNumber = 100;
// ...
static int random = (int) (Math.random() * MaxNumber + 1);
// ...
if (data > MaxNumber)
// ...
System.out.println("Only Enter Numbers Between 0-" + MaxNumber);
// ...

7. Testing for maximum attempts - == should do the work, but >= is safer: if (go >= attempt). It sounds a little paranoiac, but in complex programs and especially when dealing with multi-threading, it is possible that the variable passes over the threshold and the cycle never ends.

Final code

By merging some parts of code in a separate function, your final code could look like (not tested and no renames performed):

private static const int MaxNumber = 100;
private static int random = (int) (Math.random() * MaxNumber + 1);
private static int data;
private static int go;
private static int attempt = 10;

private static void endGuess(String message, Scanner scanInput) {
   System.out.println(message);
   scanInput.close();
}

public static boolean StartGuess() throws IOException {
    go = 0;

    while (true) {
       Scanner scanInput = new Scanner(System.in);
       data = scanInput.nextInt();

       if (go >= attempt) {
          endGuess("You have guessed the wrong number " + go + " times, sorry but you'll have to try again! :(", scanInput);
          return false;
       }

       if (data > MaxNumber ) {
          endGuess("Only Enter Numbers Between 0-" + MaxNumber, scanInput);
          continue;
       }

       if (data == random) {
          endGuess("Wow, you guessed it!!", scanInput);
          return true;
       }

       go ++;

       if (data < random)
          endGuess("Higher", scanInput);
       else // data > random 
          endGuess("Lower", scanInput);
    }
}

public static void main(String[] args) throws IOException {
    System.out.println("Guess A Number Between 0-" + MaxNumber + " Within " + attempt + " Goes");
    bool guessed = StartGuess();

    // use guessed in this context, maybe set exit code if not guessed
}

Answer 2

In addition to Alexei's feedback:

Use the Java API

You're using Math.random() and casting to integer, If you use the internal API's Random object you can forego casting by invoking Random.nextInt() which has an overload method that you can employ to retrieve a number between 1-100 by using: Random.nextInt(100) + 1;

Notes on convention

Methods in Java are usually written in camelCase, so your Start() method should simply be start() and StartGuess() would be startGuess(). This may seem of little consequence to you now, but code is the means you communicate to other programmers so following conventions matter for review, posterity and the processors which style code for readability.

Don't throw absent Exceptions

You've imported and are throwing IOException when, unless I missed something, there aren't any IO operations going on whatsoever. Get rid of them.

Answer 3

There are a bunch of things in your code wich could be optimized:

1. There is no need for static variables and constants should be made final.
2. There is no need for recursion. Always remember: If you can make it run in a loop, the loop will generally be better than a recursion. The reason is that each time you recurse, another stack frame is created. This makes stack traces hard to read and can cause stack overflow exceptions when too many levels of recursion are used.
3. Try to adhere to the Java naming schemes: Constants are SNAKE_CASE, classes are UpperCamelCase and function names and variable names are lowerCamelCase.
4. Make main() method create a game instance and let that instance perform the logic. When done, main() could set up another game on the users discretion

5. Don't use System.exit(). This is equivalent to KILL THIS PROGRAM IMMEDIATELY!!!101!. Instead, just return, when a method is done with its task. Returning from main() will gracefully exit the program.

   public class GuessingGame {
       private static final int MAX_ATTEMPTS = 10;
       private static final int MAX_NUMBER = 100;
   
       public boolean play() {
           Scanner scan = new Scanner(System.in);
           int currentAttempt = 0;
           int currentGuess = -1;
           int random = Random.nextInt(1, MAX_NUMBER);
           while (currentAttempt < MAX_ATTEMPTS && currentGuess != random) {
               currentGuess = scan.nextInt();
               if (currentGuess == -1) // Special "guess" to abort guessing
                   break; // This exits the while loop
               if (currentGuess < 1 || currentGuess > MAX_NUMBER) {
                   // Also check for negative input
                   System.out.println("Only enter numbers between 1 and " + MAX_NUMBER + ", or -1 to abort guessing.");
                   continue; // This restarts the while loop
               }
               if (currentGuess > number) {
                   System.out.println("lower");
               } else if (currentGuess < number) {
                   System.out.println("higher");
               }
               currentAttempt++;
           }
           scan.close();
           // Now we're done with guessing (either by entering -1 or reaching the limit or by guessing correctly
           if (currentGuess == -1) {
               System.out.println("Aborted guessing after " + currentAttempt + " attempts.");
           } else if (currentGuess == number) {
               System.out.println("Guessed correctly after " + currentAttempt + " attempts.");
           } else {
               System.out.println("Ran out of attempts.");
           }
           return currentGuess == number; // Return whether we guessed the number or not.
       }
   
       public static void main() {
           GuessingGame currentGame = new GuessingGame();
           currentGame.play(); // Or surround this in a loop asking for a replay, or keep statistics, or whatever.
       }
   }
   

Answer 4

Instead of infinite while loop, I'd suggest you to use attempts for keeping a check on numbers of iterations. And also I'd suggest you to return result to the calling method and display win/loss over there, as it makes your start() method more flexible to handle more cases

public class Game {
    private static int random = (int) (Math.random() * 100 + 1);
    private static int data;
    private static int go;
    private static int attempt = 10;

    public static boolean Start() {
       boolean guessed = false;
       Scanner scanInput = new Scanner(System.in);
       while (go < attempt && !guessed) {
          data = scanInput.nextInt();
          if (data > 100) {
            System.out.println("Only Enter Numbers Between 0-100");
          } else if (data == random) {
             //you can also use a break over here
             guessed = true;
          } else if (data < random) {
               go++;
               System.out.println("Higher");
          } else if (data > random) {
              go++;
             System.out.println("Lower");
          }
    }
       scanInput.close();
       return guessed;
    }

    public static void main(String[] args) throws IOException {
       System.out.println("Guess A Number Between 0-100 Within " + attempt + " Goes");
       if (Start()) {
          System.out.println("You win! Guessed it correctly");
       } else {
          System.out.println("failed to guess within num of attempts");
       }
    }
}

Answer 5

continue, break

Avoid using these control flow breaking statements. They hinder you to make refactorings like "extract method". Also code extensions are more difficult.

If you use loops (while, for) quit the loop through a condition in the loop header.

System.exit(0);

Do not use this. It's better to quit the application through the normal control flow. System.exit(x) does only make sense if you want to communicate a serious error to the calling environment (x > 0). I omitted it in the whole and let the control flow do its job.

Object scope

I prefer object scope rather than class scope. So in my example I only declared the constants as static elements.

Naming

I did some renaming to make the intention of each variable clear.

Decomposition

If you are using an OO language you should decompose long method into smaller units. That can be other methods or new classes. If you are using design patterns you always will do this. The small units can own scopes and names and my be reusable in different situations. You gain more flexibility if you keep single code fragments small although it may be verbose through all the structural code of the language.

Design patterns

Make use of design patterns. Of course for this game it looks too much. But to get an understanding about design patterns you should exersise them at an easy level like this. The question here is, will you use design patterns in complex situations correctly if you do not exercise them in easy situations.

In this approach I am using following design patterns:

1. state pattern, to represent the different states of the game
2. template method, to avoid duplication of System.out.println

Thread safety

To avoid inconsistency in multithreading environments I considered the start()-method to be synchronized. Again, this is optional for this simple exercise.

Code

import java.util.Scanner;

public class NumberGuessing {


    private static final int MAX_NUMBER = 100;
    private static final int MAX_ATTEMPTS = 10;

    private int numberToGuess;
    private int userGuess;
    private int attemptCount;

    private Scanner scanner;

    private State state; 


    private NumberGuessing() {
        this.state = new Start();
    }


    public synchronized void start() {

        while (!(this.state instanceof End)) {
            this.state.execute();
        }

    }


    /* state pattern */
    private abstract class State {

        /* template method */
        public void execute() {
            this.executeImpl();
            System.out.println(this.getStateInfo());
        }

        public abstract void executeImpl();

        public abstract String getStateInfo();

    }

    private class Start extends State {

        @Override
        public void executeImpl() {
            scanner = new Scanner(System.in);
            numberToGuess = (int) (Math.random() * MAX_NUMBER + 1);
            state = new UserInput();
        }

        @Override
        public String getStateInfo() {
            return "Guess A Number Between 0-" + MAX_NUMBER + " within " + MAX_ATTEMPTS + " tries.";
        }

    }

    private class UserInput extends State {

        @Override
        public void executeImpl() {

            userGuess = scanner.nextInt();

            if (userGuess > MAX_NUMBER) {
                state = new ExceededMaxNumber();
            } else {
                attemptCount++;
                if (userGuess == numberToGuess) {
                    state = new NumberFound();
                } else if (userGuess > numberToGuess) {
                    state = new Lower();
                } else if (userGuess < numberToGuess) {
                    state = new Higher();
                }
            }

        }

        @Override
        public String getStateInfo() {
            return "Attempt count: " + attemptCount + ".";
        }

    }

    private class End extends State {

        @Override
        public void executeImpl() {
            scanner.close();
        }

        @Override
        public String getStateInfo() {
            return "";
        }

    }

    private class NumberFound extends State {

        @Override
        public void executeImpl() {
            state = new End();
        }

        @Override
        public String getStateInfo() {
            return "Wow, you guessed it!!";
        }

    }

    private class TooMuchTries extends State {

        @Override
        public void executeImpl() {
            state = new End();
        }

        @Override
        public String getStateInfo() {
            return "You have guessed the wrong number " + attemptCount + " times, sorry but you'll have to try again! :(";
        }

    }

    private abstract class AttemptState extends State {

        @Override
        public void executeImpl() {
            if (attemptCount == MAX_ATTEMPTS) {
                state = new TooMuchTries();
            } else {
                state = new UserInput();
            }
        }

    }

    private class Higher extends AttemptState {

        @Override
        public String getStateInfo() {
            return "Higher";
        }

    }

    private class Lower extends AttemptState {

        @Override
        public String getStateInfo() {
            return "Lower";
        }

    }

    private class ExceededMaxNumber extends State {

        @Override
        public void executeImpl() {
            state = new UserInput();
        }

        @Override
        public String getStateInfo() {
            return "Only Enter Numbers Between 0-" + MAX_NUMBER;
        }

    }


    public static void main(String[] args) {
        new NumberGuessing().start();
    }

}