views:

37

answers:

2

I have a class I wrote in Java and one of the methods is getCommand() The purpose of this method is to read in a string and see what the user typed in matches any of the acceptable commands.

This is how I wrote it initially:

public char getCommand(){


    System.out.println("Input command: ");
     command = input.nextLine();

    while(command.length() != 1){
        System.out.println("Please re-enter input as one character: ");
        command = input.nextLine();
    }

    while(  command.substring(0) != "e" ||
            command.substring(0) != "c" || 
            command.substring(0) != "s" ||
            command.substring(0) != "r" ||
            command.substring(0) != "l" ||
            command.substring(0) != "u" ||
            command.substring(0) != "d" ||
            command.substring(0) != "k" ||
            command.substring(0) != "f" ||
            command.substring(0) != "t" ||
            command.substring(0) != "p" ||
            command.substring(0) != "m" ||
            command.substring(0) != "q"){
        System.out.println("Please enter a valid character: ");
        command = input.nextLine();
    }

    fCommand = command.charAt(0);

    return fCommand;

}

Now, I see the problem with this is that since I use the OR operator, it won't escape that loop because the character I type in will always not equal one of them. I tried changing it to the AND operator, but same problem. What would be the best way to only accept those specific characters? Much appreciated.

+2  A: 

Your logic is incorrect. You should be using logical ANDs and not ORs. Also I believe you want to use charAt() instead of substring() then compare characters.

i.e.,

while(  command.charAt(0) != 'e' &&
        command.charAt(0) != 'c' && 
        command.charAt(0) != 's' &&
        ...)

Otherwise if you want to test for actual single-character string inputs, just check using string equality.

while(  !command.equals("e") &&
        !command.equals("c") &&
        !command.equals("s") &&
        ...)
Jeff M
Might be a good idea to convert command to uppercase or lowercase too since he's validating the data
Coding District
Seephor
and for different cases, I just use command.toLowerCase() after reading in the string.
Seephor
A: 

You should define your commands as constants (individually). Hard coding values like this makes it harder to update your code in the future.

If the program is simply proof of concept or homework I would use:

private static final String COMMANDS = "ecsrludkftpmq";

while(!COMMANDS.contains(command.getChar(0)) {
  System.out.println("Please enter a valid character: ");
  command = input.nextLine();
}

Otherwise, if this is production code I would consider making a simple Command(char) class and providing individual command constants as part of a collection (possibly a Map against the Character key), which can be tested to see if contains a matching command.

Syntax