0

I want to put a try/catch around the input=scanner.nextInt to throw an InputMismatchException if a letter is entered, I keep getting an infinite loop. This is the code with the infinite loop. What would be best to fix this?

public void setNumberOfPlayers(Scanner scanner) {

    boolean valid = false;
    this.numberOfPlayers = 0;
    int input = 0;
    do {
        do {
            System.out.println("Please enter the number of players (minimum " + MIN_PLAYERS + " & maximum "
                    + MAX_PLAYERS + ".)");
            try {
            input = scanner.nextInt();
            }catch (InputMismatchException e ) {
                System.out.println("Please type a numeric digit only");
            }
            // this.numberOfPlayers = scanner.nextInt();
            if (input > MAX_PLAYERS || input < MIN_PLAYERS) {
                System.out.println("Invalid input. Enter the number of players");
                valid = false;
            } else {
                this.numberOfPlayers = input;
                valid = true;
            }

        } while (input > MAX_PLAYERS || input < MIN_PLAYERS);
        System.out.printf("You have chosen to include %d players. is this correct? (Y/N)", this.numberOfPlayers);

        String answer = scanner.next();

        switch (answer) {
        case "y":
        case "Y":
            System.out.println("Great - lets start the game with " + this.numberOfPlayers + " players");
            valid = true;
            break;
        case "n":
        case "N":
            System.out.println("Please re-enter the number of players");
            // this.numberOfPlayers = scanner.nextInt();
            valid = false;
            break;
        default:
            System.out.println("Please enter a valid response - y / n");
            valid = false;
            break;
        }
    } while (!valid);
}
Anton Krug
  • 1,555
  • 2
  • 19
  • 32
  • What is _throwing the loop_ supposed to mean? It's not reproducible _getting an infinite loop._ – Armali Apr 20 '21 at 20:03

1 Answers1

0

I think there are multiple problems with this approach, what if you set a valid number, but not confirm it in the second check then you will be in the first check again, entering an invalid character will no ask you again because input was set correctly the last time. So I would say you want to move the 'reset' states into the loop:

    boolean valid;
    int input;
    do {
        valid = false;
        this.numberOfPlayers = 0;
        input = 0;
        do {

Could check for 'hasNext()' on your scanner so you know you have something there. And instead of checking for exception you could test 'hasNextInt()'

 if (scanner.hasNextInt()) {
    System.out.println("Read a int" + scanner.nextInt());
 }

But overall I don't think it continues reading, the pointer/index in the scanner I think is not progressing if int was not read, because it doesn't know what is there instead and how much bytes it needs to progress to not corrupt the following longs, floats or whatever it will be there (it can't know in advance). You might give it a new input, but the reader still didn't process validly the previous one, so it's not progressing.

So I would say better would be to read a line 'nextLine()' and be sure it was read correctly no matter what and only then try to convert a string into an int and then figure out if the input is what you want.

Other problem I see is that there are kinda overlapping functionality with input and valid flag, conditions test multiple times for valid range of the input (inside a if condition and inside a while condition, duplicate code will lead to bugs). Maybe the while condition could be checking for the valid flag instead:

Changing:

while (input > MAX_PLAYERS || input < MIN_PLAYERS);

To this:

while (!valid);

And I'm not sure if this.numberOfPlayers = 0; is good as well, the setNumberOfPlayers implies that you will set it to valid value, so why you are resetting in the meantime, when the two loops will be finished then set it to the next new valid value, do not reset it into an invalid value in the meantime. I guess here it's not as big problem as you are not multithreading, but still it doesn't feel right.

Anton Krug
  • 1,555
  • 2
  • 19
  • 32