-1

I'm trying to get user input and check if it is "1" or "2" and display an error messages if it's not. I keep getting error messages even when the input is correct.

Scanner user_input = new Scanner(System.in);
String choice = "";
// Input Validation
do {
   // Read user choice
   choice = user_input.nextLine();
            
   if (!choice.equals("1") || !choice.equals("2"))
      System.out.println("Invalid input. Give new value");
   }while (!choice.equals("1") || !choice.equals("2"));```

panosttm
  • 1
  • 2
  • 1
    Think about exactly what `!choice.equals("1") || !choice.equals("2")` means. `a || b` is false when both `a` and `b` are false. When will both of your !equals checks be false? – tgdavies Jan 16 '22 at 22:27
  • Other then maybe using a `boolean` flag to determine correctness (for the loop), what's wrong with your current approach? – MadProgrammer Jan 16 '22 at 22:27
  • @tgdavies When I give input "1", which make the condition true, it still refuses to stop the loop – panosttm Jan 16 '22 at 22:33
  • @panosttm just FYI validating command line input isn't (IMO) the best use of your learning time. HTML5's input validation is really the state of the art. And most of the time these days you'll mostly likely be using Java on the server side, where you can use a 400 error to say "bad input" – ControlAltDel Jan 16 '22 at 22:33
  • The loop will exit when the condition is `false`, not when it's `true`. – tgdavies Jan 16 '22 at 22:34
  • Hi Guy, Please replace || by && . The problem is with your logic – elgsylvain85 Jan 16 '22 at 22:40

4 Answers4

1

Your condition is incorrect. Use logical AND if need to eliminate both 1 and 2. I think you wanted to achieve this

       do {
            choice = user_input.nextLine();

            if (!choice.equals("1") && !choice.equals("2"))
                System.out.println("Invalid input. Give new value");
        } while (!choice.equals("1") && !choice.equals("2"));

Also to remove redundancy and improve the readability of code consider removing the validation logic to a separate method.

public static void main(String[] args) {
        Scanner user_input = new Scanner(System.in);
        String choice = "";
        
        do {
            choice = user_input.nextLine();

            if (isValid(choice))
                System.out.println("Invalid input. Give new value");
        } while (isValid(choice));

        System.out.println("Your input is valid: " + choice);
    }
    
    private static boolean isValid(String choice) {
        return !choice.equals("1") && !choice.equals("2");
    }
Alexander Ivanchenko
  • 25,667
  • 5
  • 22
  • 46
0

if (!choice.equals("1") || !choice.equals("2")) doesn't make sense

I'm pretty simple minded, so I always like to do some simple checks, for example...

System.out.println(!true || !false); // !"1".equals("1") || !"1".equals("2")
System.out.println(!false || !true); // !"2".equals("1") || !"2".equals("2")
System.out.println(!false || !false); // !"3".equals("1") || !"3".equals("2")

Which prints

true // Show error message ... this is wrong, `1` is valid
true // Show error message ... this is wrong, `2` is valid 
true // Show error message ... this is correct

which is obvious wrong, the first statement, we when not 1 or 2, print an error message

So, instead, we should invert the result of both sides of the ||, for example...

System.out.println(!(true  || false)); // !("1".equals("1") || "1".equals("2"))
System.out.println(!(false  || true)); // !("2".equals("1") || "2".equals("2"))
System.out.println(!(false || false)); // !("3".equals("1") || "3".equals("2"))

which prints...

false // Don't show error message ... `1` is valid
false // Don't show error message ... `2` is valid
true // Show error message ... `3` is invalid

Okay, that seems better

I'd also simplify the exit condition along the way...

Scanner user_input = new Scanner(System.in);
String choice = "";
boolean isValidChoice = false;
// Input Validation
do {
    // Read user choice
    choice = user_input.nextLine();

    if (!(choice.equals("1") || choice.equals("2"))) {
        System.out.println("Invalid input. Give new value");
    } else {
        isValidChoice = true;
    }
} while (!isValidChoice);
System.out.println("Good choice!");
MadProgrammer
  • 343,457
  • 22
  • 230
  • 366
0

There's a logical error in the conditions. There should be "&&" instead of "||".

Thanks to @tgdavies.

Like this...

        String choice = "";
        // Input Validation
        do {
           // Read user choice
           choice = user_input.nextLine();

           if (!choice.equals("1") && !choice.equals("2"))
              System.out.println("Invalid input. Give new value");
        }while (!choice.equals("1") && !choice.equals("2"));
panosttm
  • 1
  • 2
0

You can escape in the middle of the loop if the input is valid.

public static void main(String[] args) {
    Scanner user_input = new Scanner(System.in);
    String choice = "";
    // Input Validation
    while (true) {
        // Read user choice
        choice = user_input.nextLine();
        if (choice.equals("1") || choice.equals("2"))
            break;
        System.out.println("Invalid input. Give new value");
    }
}