Condensing basic code using || operator with a string

Question

I'm quite new to java programming. I was unable to find any information relating to the use of the || operator with strings. I was wondering if there was a more efficient way to do this code in particular that was still easily readable. I tried making a simple calculator as a way to familiarize myself with IfThenElse statements.

    import java.util.Scanner;
     public class SimpleCalculator {
        public static void main(String[] args){
            Scanner input=new Scanner(System.in);
            double first;
            double second;
            String option;

            while(true){
                System.out.println("What function would you like to calculate?");
                option=input.next();    
                    if(option.equals("add") || option.equals("+")){
                        System.out.println("First number");
                        first=input.nextDouble();
                        System.out.println("Second number");
                        second=input.nextDouble();
                        double add=first+second;
                        System.out.println(add);
                    }
                    else if(option.equals("subtract") || option.equals("-")) {
                        System.out.println("First number");
                        first=input.nextDouble();
                        System.out.println("Second number");
                        second=input.nextDouble();
                        double subtract=first-second;
                        System.out.println(subtract);
                    }
                    else if(option.equals("multiply") ||option.equals("*")) {
                        System.out.println("First number");
                        first=input.nextDouble();
                        System.out.println("Second number");
                        second=input.nextDouble();
                        double multiply=first*second;
                        System.out.println(multiply);
                    }
                    else if(option.equals("divide") || option.equals("/"))  {
                        System.out.println("First number");
                        first=input.nextDouble();
                        System.out.println("Second number");
                        second=input.nextDouble();
                        double divide=first/second;
                        System.out.println(divide);
                    }
                    else if(option.equals("end")){
                        System.exit(0);
                }
            }
        }
    }

For the most part I am wondering about the if requirements, which I have tested and they do work, but it seems a bit clunky to me. However, any critique would be greatly appreciated.

equals returns boolean so you are not use || with String anyway — Kalpesh Soni, Sep 19 '14 at 01:37

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

Switch/case statements are a nice alternative to a series of ifs, and as of Java 7 you can use switch statements with strings. Note the syntactical difference between the two. Instead of grouping things with curly braces, each case ends with a break statement.

switch (option) {
    case "add":
    case "+":
        System.out.println("First number");
        first=input.nextDouble();
        System.out.println("Second number");
        second=input.nextDouble();
        double add=first+second;
        System.out.println(add);

        break;

    case "subtract":
    case "-":
        System.out.println("First number");
        first=input.nextDouble();
        System.out.println("Second number");
        second=input.nextDouble();
        double subtract=first-second;
        System.out.println(subtract);

        break;

    case "multiply":
    case "*":
        System.out.println("First number");
        first=input.nextDouble();
        System.out.println("Second number");
        second=input.nextDouble();
        double multiply=first*second;
        System.out.println(multiply);

        break;

    case "divide":
    case "/":
        System.out.println("First number");
        first=input.nextDouble();
        System.out.println("Second number");
        second=input.nextDouble();
        double divide=first/second;
        System.out.println(divide);

        break;

    case "end":
        System.exit(0);
}

I would then suggest combining the duplicated prompt code. If you find yourself copying and pasting code it's usually a good idea to take a step back and figure out how you can avoid the repetition. Duplicated code is a sign that you should do some refactoring.

if (option.equals("end")) {
    System.exit(0);
}

System.out.println("First number");
first=input.nextDouble();
System.out.println("Second number");
second=input.nextDouble();

switch (option) {
    case "add":
    case "+":
        double add=first+second;
        System.out.println(add);
        break;

    case "subtract":
    case "-":
        double subtract=first-second;
        System.out.println(subtract);
        break;

    case "multiply":
    case "*":
        double multiply=first*second;
        System.out.println(multiply);
        break;

    case "divide":
    case "/":
        double divide=first/second;
        System.out.println(divide);
        break;
}

Furthermore, you could also eliminate the duplicate printouts by using a single result variable for all of the calculations.

if (option.equals("end")) {
    System.exit(0);
}

System.out.println("First number");
first=input.nextDouble();
System.out.println("Second number");
second=input.nextDouble();

double result;

switch (option) {
    case "add":      case "+": result = first + second; break;
    case "subtract": case "-": result = first - second; break;
    case "multiply": case "*": result = first * second; break;
    case "divide":   case "/": result = first / second; break;
}

System.out.println(result);

Very nice, but I would like to add that you should convert option to lower case before comparing it in the switch statement. Also a default case should be used as well (result is undefined otherwise). — nmore, Sep 19 '14 at 01:47
Thank you very much, this has helped alot! I did specifically want it so that the second number was entered after the function was chosen. Is there a way to do that without repeating it within each possibility? — Aerebes, Sep 20 '14 at 02:07

Aviv Cohn · Answer 2 · 2014-09-20T20:21:26.507

You're use of || seems fine to me. However I have a number of general suggestions to make the code better overall.

First of all, why not have isAdd, isSubtract, etc. functions? For example:

private boolean isAdd(String input){
    return input.equalsIgnoreCase("add") || input.equals("+");
}

Same goes for the other operators. Than you can have code such as:

if (isAdd(option)) ...

Which is more readable than

if (input.equalsIgnoreCase("add") || input.equals("+")) ...

In a larger program, you might need to check these kinds of things more than once, and then having a method to do this becomes extra-handy. Also, this way if you want to change the definition of "add" (for example now "a" also qualifies), you change code in one place and the whole program complies.

Secondly, why not extract the bodies of these if statements into other functions? Than your code would read like so:

if (isAdd(option))
    performAddition();
else if (isSubtract(option))
    performSubtraction();
// .. etc

// function definitions here

Making for a much more readable program, as opposed to what you currently have.

Thirdly, notice where you put your spaces. option = input.next() looks better than option=input.next().

That's it pretty much. Good luck :)

Why use `.toLowerCase()` when you can use `.equalsIgnoreCase()`? — Pokechu22, Sep 19 '14 at 01:36

Peerhenry · Answer 3 · 2014-09-19T02:26:06.450

John Kugelman and Aviv Cohn both gave good advice. I would like to add that your application will throw an InputMismatchException if you don't enter a valid number at the call to nextDouble(). Instead of your program terminating because of the exception you can prompt the user to enter a valid number after which he/she can try again.

One way to do this is by adding the following methods to SimpleCalculator:

    private static Double getValidNumber()
    {
        Double nr = null;
        while( nr == null )
        {
            nr = getNextDouble();
            if(nr == null) System.out.println("Please enter a valid number.");
        }
        return nr;
    }

    private static Double getNextDouble()
    {
        Scanner input=new Scanner(System.in);
        Double output = null;
        try{ output = input.nextDouble(); }catch(InputMismatchException e){}
        return output;
    }

Then in your main method, simply replace the calls to input.nextDouble() by getValidNumber().

An error comes up when I try to compile through the command line: error:cannot find symbol try{ output = input.nextDouble(); }catch(InputMismatchException e){} symbol: class InputMismatchException location: class SimpleCalculator — Aerebes, Sep 20 '14 at 02:35
I ended up fixing the error by switching from catch(InputMismatchException) to catch(Exception) I think the compiler for some reason did not read InputMismatchException as a valid error — Aerebes, Sep 20 '14 at 02:52

Condensing basic code using || operator with a string

3 Answers3