1

I'm trying to make this Sentinel program more robust by continuing it even when incorrect user inputs are received. I've gotten it to work if the user input is a different int, but if it is a string, or anything else really, the program crashes.

My current code attempt is this:

 } else if (userInt != 1 && userInt != 2 && userInt != 3 && userInt != 4 && userInt != 5 && userInt !=6
            || userInt instanceof String) {

The first part of this code works fine at checking if the user input is a different in. The instanceof statement gives the error of "incompatible operand types int and String"

Should I even be using an instanceof statement? Is there a better way to check for this?

This is the whole method:

public static void printMenu() {

    Scanner userInput2 = new Scanner(System.in);

    String menu = new String("    Please choose from the following menu: \n    1. Rock paper Scissors\n    2. "
            + "Tip Calculator\n    3. "
            + "Number Adding\n    4. Guessing Game\n    5. Random\n    6. Exit");
    System.out.println(menu);
    int userInt = userInput2.nextInt();

    if (userInt == 1) {
        System.out.println("    You asked to play Rock Paper Scissors");
        System.out.println("    Launching Rock Paper Scissors... \n");
        RockPaperScissors gameRun1 = new RockPaperScissors();
        gameRun1.main(null);
    } else if (userInt == 2) {
        System.out.println("    You asked to run the Tip Calculator");
        System.out.println("    Launching the Tip Calculator... \n");
        TipCalculator gameRun2 = new TipCalculator();
        gameRun2.main(null);
    } else if (userInt == 3) {
        System.out.println("    You asked to run the Number Adding game");
        System.out.println("    Launching the Number Adding game... \n");
        NumberAddingGame gameRun3 = new NumberAddingGame();
        gameRun3.main(null);
    } else if (userInt == 4) {
        System.out.println("    You asked to play GuessingGame");
        System.out.println("    Launching GuessingGame... \n");
        GuessingGame gameRun4 = new GuessingGame();
        gameRun4.main(null);
    } else if (userInt == 5) {
            System.out.println("    You asked for a random game");
            option5();
    } else if (userInt == 6) {
            System.out.println(    "Thank you for using Conner's Sentinel");
            // figure out how to terminate the program from here

    } else if (userInt != 1 && userInt != 2 && userInt != 3 && userInt != 4 && userInt != 5 && userInt !=6
            || userInt instanceof String {
        System.out.println("Not a valid input, type 1-6");
        printMenu();
    }
    printMenu();
}
  • 1
    This is not directly apropos to your question, but a `switch` statement would be neater than a chain of `if (userInt == ` tests. – Stephen C May 14 '18 at 22:53
  • right, and remove the recursion too. – YoYo May 14 '18 at 23:13
  • I'm currently taking an intro computer science class and the teacher only wants us to use stuff we have already covered. Have not covered switch statements yet. – Bill Bumbleton May 14 '18 at 23:30

4 Answers4

2

There is no way to check if the next input was an int like you are doing (userInput2.nextInt() can only return an int), instead you have to check before you assign the result. Something like,

if (userInput2.hasNextInt()) {
    int userInt = userInput2.nextInt();
    if (userInt == 1) {
        System.out.println("    You asked to play Rock Paper Scissors");
        System.out.println("    Launching Rock Paper Scissors... \n");
        RockPaperScissors gameRun1 = new RockPaperScissors();
        gameRun1.main(null);
    } else if (userInt == 2) {
        System.out.println("    You asked to run the Tip Calculator");
        System.out.println("    Launching the Tip Calculator... \n");
        TipCalculator gameRun2 = new TipCalculator();
        gameRun2.main(null);
    } else if (userInt == 3) {
        System.out.println("    You asked to run the Number Adding game");
        System.out.println("    Launching the Number Adding game... \n");
        NumberAddingGame gameRun3 = new NumberAddingGame();
        gameRun3.main(null);
    } else if (userInt == 4) {
        System.out.println("    You asked to play GuessingGame");
        System.out.println("    Launching GuessingGame... \n");
        GuessingGame gameRun4 = new GuessingGame();
        gameRun4.main(null);
    } else if (userInt == 5) {
        System.out.println("    You asked for a random game");
        option5();
    } else if (userInt == 6) {
        System.out.println("Thank you for using Conner's Sentinel");
        // figure out how to terminate the program from here
    } else {
        System.out.println("Not a valid input, type 1-6");
        printMenu();
    }
} else {
    userInput2.nextLine(); // <-- consume the non-number
    System.out.println("Not a valid number, type 1-6");
    printMenu();
}
Elliott Frisch
  • 198,278
  • 20
  • 158
  • 249
2

Instead of "expecting" an int...

int userInt = userInput2.nextInt();

You should "expect" a String...

int actualInput = userInput2.nextLine();

From this you could then use Integer.parseInt(String) in an attempt to parse the String to an int, this will give you your first chance to validate the value.

The problem with this is Integer.parseInt(String) can throw a NumberFormatException, and you really should avoid making logic decisions based on exceptions.

Another approach might be to use a regular expression instead, something like...

if (actualInput.matches("^\\d*")) {
    // This is a number, safe to parse to int
} else {
    // This is not a number and is not safe to be parsed
}

Once you're satisfied that the actualInput is a number, you could use another Scanner to get the next int...

Scanner safeScanner = new Scanner(actualInput);
int userInt = safeScanner.nextInt();

as an example

MadProgrammer
  • 343,457
  • 22
  • 230
  • 366
1

userInt instanceof String will always be false, since userInt is an int. If it is an int, you don't need to check for instanceof string.

What you meant is to proof check the string from user input with StringUtils.isNumeric and reduce your other expressions to: if 1<= userInt && userInt <=6

Should I even be using an instanceof statement? Is there a better way to check for this?

Even if some other string, non numeric, could be converted to int and result into a value between 1 and 6, this way it would be rejected. Keep the numeric check, yes, just the correct one, StringUtils.isNumeric .

If you opt to have userInt as String, instead, then turn the other expression into if 1<= Integer.parseInt(userInt) && Integer.parseInt(userInt) <=6

Attersson
  • 4,755
  • 1
  • 15
  • 29
0

First of all, after you write something like this:

int userInt = userInput2.nextInt();

your userInt is declared as an int, and can be nothing but a int. So writing something like this makes no sense:

userInt instanceof String

Because here, you already know that userInt is an int, because you declared it so.

The problem (where the exception, or crash as you called it, occurred) is elsewhere. It will happen at the call to nextInt().

Read the documentation for Scanner.nextInt(), under the exceptions, it states:

Throws: InputMismatchException - if the next token does not match the Integer regular expression, or is out of range

So that is exactly what happens.

You have several choices, two of which:

  1. catch the exception, and handle it the way you want it to be handled
  2. Use of Scanner.hasNextInt().

I already see a growing number of alternative approaches.

Also most people would translate that chain of if/else if statements into a switch/case/default construct.

Then an other problem in your printMenu() method, it is endlessly recursive. Even though it is just user input, and it might have a limited timespan, with limited user entries before it exits, in theory you could reach a situation where you get a StackOverflowException. This implementation begs to be converted from recursive to iterative to avoid overallocation of objects (memory leak) and having a stack that grows forever.

YoYo
  • 9,157
  • 8
  • 57
  • 74