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!");