Input validation in Java

Question

I have tried plenty of times to get this to work. I want to make it so if a user inputs anything other than what is meant to be input, then it gives them an error message and prompts them to input a new answer. However, every time I attempt this, it displays an error message (my error message) no matter what I input, even if it's correct. Help?

import java.util.Random;
import java.util.Scanner;
public class RandomSelect 
{
    public static void main(String[] args) 
    {
         String [] arr = {"rock", "paper", "scissors"};
         Random random = new Random();
         Scanner scan = new Scanner(System.in);
         System.out.println("Please select: rock, paper, or scissors?");
         String myChoice = scan.nextLine();
         boolean myChoiceIsntCorrect = false;
         if ((!myChoice.equalsIgnoreCase("rock")) || (!myChoice.equalsIgnoreCase("paper")) || (!myChoice.equalsIgnoreCase("scissors")))
         {
            myChoiceIsntCorrect = true;
         }
         while ( myChoiceIsntCorrect == true )
         {
            System.out.println("Your input wasn't either rock, paper, or scissors. Please select one of the 3.");
            myChoice = scan.nextLine();
         }

         int select = random.nextInt(arr.length); 
         System.out.println("Computer selected " + arr[select] + ".");

         if (arr[select] == "rock" && myChoice.equalsIgnoreCase("paper"))
            System.out.println("You win!");
         if (arr[select] == "rock" && myChoice.equalsIgnoreCase("scissors"))
            System.out.println("You lose!");
         if (arr[select] == "rock" && myChoice.equalsIgnoreCase(arr[select]))
            System.out.println("It's a tie!");
         if (arr[select] == "paper" && myChoice.equalsIgnoreCase("rock"))
            System.out.println("You lose!");
         if (arr[select] == "paper" && myChoice.equalsIgnoreCase("scissors"))
            System.out.println("You win!");
         if (arr[select] == "paper" && myChoice.equalsIgnoreCase(arr[select]))
            System.out.println("It's a tie!");
         if (arr[select] == "scissors" && myChoice.equalsIgnoreCase("paper"))
            System.out.println("You lose!");
         if (arr[select] == "scissors" && myChoice.equalsIgnoreCase("rock"))
            System.out.println("You win!");
         if (arr[select] == "scissors" && myChoice.equalsIgnoreCase(arr[select]))
            System.out.println("It's a tie!");
    }
}

I have attempted it without the boolean, and thought the boolean might work. But it didn't work with or without. What am I doing wrong? And yes, I am a newbie to java. I am taking a java course at school atm.

score 0 · Accepted Answer · edited May 23 '17 at 12:05

Take

if ((!myChoice.equalsIgnoreCase("rock")) || (!myChoice.equalsIgnoreCase("paper")) || (!myChoice.equalsIgnoreCase("scissors"))) {
    myChoiceIsntCorrect = true
}

and put it into words:

If my choice isn't "rock" or if my choice isn't "paper" or if my choice isn't "scissors", my choice isn't correct.

So now set your choice to "rock".

My choice is "rock". My choice isn't "paper". So set that my choice isn't correct.

Use &&.

And read: How do I compare strings in Java?

score 0 · Answer 2 · answered Oct 11 '14 at 01:15

0

if ((!myChoice.equalsIgnoreCase("rock")) || (!myChoice.equals("paper")) || (!myChoice.equals("scissors")))
{
   myChoiceIsntCorrect = true;
}

The problem is in that if statement, you're saying if the choice is not rock OR not paper OR not scissors. When the user enters one of them, it fails the other 2.

Try using && signs instead.

answered Oct 11 '14 at 01:15

Thanks guys, that fixed my issue! It makes more sense using or, though, in my opinion. – coreynj Oct 11 '14 at 01:20
You need to think about the code you have written though. If it's not rock or it's not paper or it's not scissors When you choose rock, you only pass 1/3 of them tests. It will fail the other 2 – Oct 11 '14 at 02:00

score 0 · Answer 3 · answered Oct 11 '14 at 01:25

There are two problems with your input validation, the first being that, as others pointed out, you need to use && not || because by using if my input is not equal to a or not equal to b or not equal to c, you are saying that the input has to be equal to a,b, and c since if it isn't equal to any of them, you'll get a true value. The second problem is that you never change myChoiceIsntCorrect inside the loop, which means that once inside that loop, myChoiceIsntCorrect will always evaluate to true and your program will keep looping forever.

score 0 · Answer 4 · answered Oct 11 '14 at 01:26

There are two problems here.

First, your while loop will run forever because you're not updating myChoiceIsntCorrect inside the loop. You should change it to something like this:

boolean invalidChoice = true;
do {
    myChoice = scan.nextLine();
    if ((!myChoice.equalsIgnoreCase("rock")) || (!myChoice.equals("paper")) || (!myChoice.equals("scissors"))) {
        System.out.println("Your input wasn't either rock, paper, or scissors. Please select one of the 3.");
    } else {
        invalidChoice = false;
    }
} while ( invalidChoice );

Do note that you can simplify the if here by inverting the if and the else clauses.

Second, you're comparing string with ==. In java, == compares references. Something like "thing" == "thing" will always return false, because the two "thing" are different object. Your ifs with arr[select] == "..." need the be replaced with "...".equals(arr[select]).

score 0 · Answer 5 · answered Oct 11 '14 at 01:29

You've got several things going on here.

If this block of code were correct, it would result in an infinite loop. Observe:

boolean myChoiceIsntCorrect = false;
 if ((!myChoice.equalsIgnoreCase("rock")) || (!myChoice.equalsIgnoreCase("paper")) || (!myChoice.equalsIgnoreCase("scissors")))
 {
    myChoiceIsntCorrect = true;
 }
 while ( myChoiceIsntCorrect == true )
 {
    System.out.println("Your input wasn't either rock, paper, or scissors. Please select one of the 3.");
    myChoice = scan.nextLine();
 }

You never update myChoiceIsntCorrect.

Instead, consider moving the check logic into another method, and change the loop to a do...while, so you can get the benefit of asking coupled with the benefit of error checking. You'll also want to fix your boolean; here, I've changed it to be affirmative as opposed to negative (which would lead to confusion).

private boolean valid(String word) {
    return myChoice.equalsIgnoreCase("rock") ||
           myChoice.equalsIgnoreCase("paper") ||
           mychoice.equalsIgnoreCase("scissors");
}

// block of code to follow
String myChoice;
do {
    myChoice = scan.nextLine();
} while(!valid(myChoice));

You check strings correctly in one instance, but not in another:

if (arr[select] == "rock" && myChoice.equalsIgnoreCase("paper"))

That simply ain't going to work.

You could fix the broken comparison, or make it read a bit better instead with a switch statement, and move your actual boolean comparison logic into another method.

switch(select) {
    case 0: // Rock
        checkWinCondition(arr[0], myChoice);
        break;
    // other cases and default with a good error message
}

private boolean checkWinCondition(String computerSelected, String userSelected) {
    if(computerSelected.equalsIgnoreCase(userSelected)) {
        // draw
    } else {
        // other conditions
    }
}

All of the approaches above are meant to improve readability and clarity of intent. If you don't want to make the latter clearer (i.e. you only want to fix the bad == comparison), then that's fine.

Input validation in Java

5 Answers5