CODEWITHSUNDEEP
javaif-statement
GUST
GUST

Reputation: 33

Is there a simpler way to write this chain of equalsIgnoreCase checks?

I am attempting to write a method that takes user input in the form of a string, and then returns an int value based on what the string contained/equals. I have already wrote a method that works, but it is quite long, and I was wondering if there was a way to make it shorter?

This is the method in its current state:

private int readCommandInput(){
    System.out.print("Command?>");
    String userCommand = programAssignment.assignmentScanner.nextLine();
    if(userCommand.equalsIgnoreCase(registerNewDogCMD)){ return 1; }
    else if(userCommand.equalsIgnoreCase(listDogsCMD)){ return 2; }
    else if(userCommand.equalsIgnoreCase(increaseDogAgeCMD)){ return 3; }
    else if(userCommand.equalsIgnoreCase(removeDogCMD)){ return 4; }
    else if(userCommand.equalsIgnoreCase(registerNewOwnerCMD)){ return 5; }
    else if(userCommand.equalsIgnoreCase(giveDogCMD)){ return 6; }
    else if(userCommand.equalsIgnoreCase(listOwnersCMD)){ return 7; }
    else if(userCommand.equalsIgnoreCase(removeOwnerCMD)){ return 8; }
    else if(userCommand.equalsIgnoreCase(startAuctionCMD)){ return 9; }
    else if(userCommand.equalsIgnoreCase(makeBidCMD)){ return 10; }
    else if(userCommand.equalsIgnoreCase(listBidsCMD)){ return 11; }
    else if(userCommand.equalsIgnoreCase(listAuctionsCMD)){ return 12; }
    else if(userCommand.equalsIgnoreCase(closeAuctionsCMD)){ return 13; }
    return 0;
}

Some extra information:

Upvotes: 2

Views: 172

Answers (2)

dreamcrash
dreamcrash

Reputation: 51483

For your specific use-case you have two options:

  1. use an array and its indices;
  2. use a Map (overall the better approach).

First Option:

Since, in your case, the values being returned are continuous ints from 1 to N (i.e., 1, 2, .., N), without any duplicates, you can simply turn into an array all the strings to be compared against, iterate over that array, and use its index as the returning value.

First create an array with the commands:

String[] commands = {"registerNewDogCMD", ...., "closeAuctionsCMD"}

you can make the command strings already lower case during their declaration, otherwise you might have lower then afterwards.

Now adapt the read command method:

private int readCommandInput(String [] commands){
    System.out.print("Command?>");
    String userCommand = programAssignment.assignmentScanner.nextLine();

    for(int i = 0; i < vars.length; i++)
       if(userCommand.equalsIgnoreCase(vars[i]))
          return i + 1;
   return 0;
}

We can even make it simpler by using Arrays.asList and indexOf methods:

   private static int readCommandInput(String[] commands){
       ...
       return Arrays.stream(commands).map(String::toLowerCase)
                                     .collect(Collectors.toList())
                                     .indexOf(userCommand) + 1;
  }

The second option:

This option (i.e., using a Map) is the most appropriate, and the most suitable for use-cases that the values being returned are not continuous ints or can have duplicates. It would look as follows (using Java 9):

Map<String, Integer> commands = Map.of(entry("registernewdogcmd", 1),...,  entry("closeauctionscmd", 13));

user case:

 private static int readCommandInput(Map<String, Integer> commands, int default_value){
       System.out.print("Command?>");
       String userCommand = programAssignment.assignmentScanner.nextLine().toLowerCase();
       return commands.getOrDefault(userCommand, default_value);
  }

This second approach, for your use-case, results in a bit more code and memory usage (i.e., having to explicitly defined the keys), however, the benefits are:

  1. faster because you can access the Key (i.e., user command) with complexity O(1), whereas with the array approach you have a complexity of O(N) in the worst-case.
  2. it makes the mapping (between commands and returned values) more explicit;
  3. it is more flexible to future code changes, for instance, if in the future you change your code to also return non-continuous values (or to use string values instead of ints) with the map approach you could easily adapt your code (i.e., adding the new commands/returning values or changing the values).

Further Improvements:

IMO you could further improve your code by moving the command-related concerns into an Enum, which would increase readability, and separation of concerns. The design would look like as follows:

public enum Commands {
    REGISTER_NEW_DOG_CMD(1, "registerNewDogCMD"),
    ....
    CLOSE_AUCTION_CMD(13, "closeAuctionsCMD");

    private static final int COMMAND_DOES_NOT_EXIST = 0;
    private static final Map<String, Integer> LOOK_UP = new HashMap<>(Commands.values().length);

    static{
        Arrays.stream(Commands.values())
              .forEach(c -> LOOK_UP.put(c.getName().toLowerCase(), c.getValue()));
    }

    private final int value;
    private final String name;

    Commands(int value, String name) {
        this.value = value;
        this.name = name;
    }

    public int getValue() {
        return value;
    }

    public static int getValue(String userCommand) {
        return LOOK_UP.getOrDefault(userCommand.toLowerCase(), COMMAND_DOES_NOT_EXIST);
    }

    public String getName() {
        return name;
    }
}

Then you would change your method to:

   private int readCommandInput(){
        System.out.print("Command?>");
        String userCommand = programAssignment.assignmentScanner.nextLine();
        return Commands.getValue(userCommand);
    }

Ideally, you would change the signature to return an Enum Command type instead of a int.

Upvotes: 4

mEstrazulas
mEstrazulas

Reputation: 178

For me, the best approach for this case is:

String userCommand = programAssignment.assignmentScanner.nextLine().toLowerCase();

HashMap<String, Integer> hmap = new HashMap<String, Integer>(); 
hmap.put("aa", 1);
hmap.put("bb", 2);
hmap.put("cc", 3);
hmap.put("dd", 4);
return hmap.getOrDefault(userCommand, 0);

Upvotes: 9

Related Questions