Simple Java calculator

Question

I am a beginner in Java programming. Here is my simple calculator. I hope you could scan through my coding and let me know if there is any error in my coding or how I could simplify the code.

import java.util.Scanner;

public class Decimal {
    /**
     * @param args
     */
    public static void main(String[] args) {
        double n1, n2;
        String operation;
        Scanner scannerObject = new Scanner(System.in);

        System.out.println("Enter first number");
        n1 = scannerObject. nextDouble();

        System.out.println("Enter second number");
        n2 = scannerObject. nextDouble();

        Scanner op = new Scanner(System.in);
        System.out.println("Enter your operation");
        operation = op.next();

        switch (operation)  {
        case "+":
            System.out.println("Your answer is " + (n1 + n2));
            break;

        case "-":
            System.out.println("Your answer is " + (n1 - n2));
            break;

        case "/":
            System.out.println("Your answer is " + (n1 / n2));
            break;

        case "*":
            System.out.println("Your asnwer is " + (n1 * n2));
            break;

        default:
            System.out.println("Je ne sais pas");

        }
    }
}

Answer 1

Looks good for a beginner.

Couple tweaks if you want to make it look nicer. The name of the class should be somewhat describing the purpose so in your case Calculator would fit better. If you are familiar with enum you could parse your +, -, / into an enum and do switch on that. Then next tweak could be to do the System.printout at the end and in the switch case to do just the operation.

Another suggestion would be to make 4 classes implementing binary operations with just one method taking two arguments and returning the result. Then you can have a map with a key being the operation (in enum or string) and the value would be the appropriate class. Then you end up with more classes but this main method gets shorter.

Generally it is better to split the code in more methods and classes and keep the motto "one class one responsibility", or Single Responsibility Principle. So as an example each of your arithmetic classes would be responsible for a single arithmetic operation. If you want to follow that completely, then you would have to create a class that gets the input from the user as well. The Calculator class then would be just like a coordinator saying: give me numbers, give me an operator, perform the operation and print out the result at the end.

Note: If you want some sample code leave me a comment here and I can add some.

Answer 2

This is a really neat program. For what it does, it does a good job.

Really, there’s only one point of criticism, and a few suggestions. The criticism is designed to get you in to the right habits, it’s not a major bug …

Using try‐with‐resources

For years Java has had a problem with people being lazy about closing resources … you open a file, read its contents, and move on … leaving an open file handle and other resources until the garbage collector cleans up the mess. These ‘forgotten’ resources can sometimes lead to unexpected bugs and deadlocks.

In Java7, the concept ‘try‐with‐resources’ was added to ensure that resources are guaranteed to be closed nicely whether the code succeeds or not. It actually allows the programmer to be even lazier than before, and get an even better result.

In your case, the Scanner instance is a resource that should be closed …

public static void main(String[] args) {
    try (Scanner scannerObject = new Scanner(System.in)) {
        double n1, n2;
        String operation;

        ……

    }
}

OK, that’s the right way to use the Scanner.

Further Improvements

As your program expands, you will find that there are a few things that become ‘uncomfortable’ … the number of operators will increase, and the complexity of the calculations will increase as well … eventually you will want to enter expressions like 100 * ( 17 / 20 ) to get the percent score if you got 17 out of 20 in a test … etc.

What you have at the moment is great for its purpose, but I encourage you to try to find ways to expand on the idea.

Research you may want to do is in to things like Reverse Polish Notation (RPN) which is an easier‐to‐parse format for writing mathematical expressions. Then you can look at things that help you convert infix to postfix (RPN) notation …

In order to do these more expensive operations you will need to find ways to turn operators in to classes (probably an Enum), and have classes that allow you to encapsulate an expression (unary or binary expressions).

Good job on this, otherwise.

Answer 3

Not bad for a beginner.

You shouldn't have to instantiate more than one scanner, as your numbers and the operation are coming from the same input stream. You can even accept your input in a more human-friendly way, as one expression.

There's no need to name your scanner somethingObject — most of your variables will be objects!

You have a typo ("Your asnwer is "), which illustrates the problem with repetitive code. I'll leave it as an exercise for you to figure out a way to restructure your program to eliminate the repetitive System.out.println() calls.

I'd rename the class to something more descriptive.

public class Calculator {
    public static void main(String[] args) {
        try (Scanner scanner = new Scanner(System.in)) {
            System.out.println("Enter an expression of the form 3 * 5");
            double n1 = scanner.nextDouble();
            String operation = scanner.next();
            double n2 = scanner.nextDouble();

            switch (operation)  {
            case "+":
                System.out.println("Your answer is " + (n1 + n2));
                break;

            case "-":
                System.out.println("Your answer is " + (n1 - n2));
                break;

            case "/":
                System.out.println("Your answer is " + (n1 / n2));
                break;

            case "*":
                System.out.println("Your asnwer is " + (n1 * n2));
                break;

            default:
                System.out.println("Je ne sais pas");

            }
        }
    }
}

Answer 4

Good job. Some points that can be improved:

• As already mentioned by the others, you could extract the computation logic to separate methods.

• You repeat System.out.println("Your answer is " + [...]); several times in your code. Code repetition is bad. To fix it (if you want to keep the switch-case block), you could store the result of the computation into an integer variable and print the result after the switch-case block.

• I personally don't declare the variables at the very beginning of the method. I declare them when I need them for the first time. As a consequence, their scope is reduced.

• You create a new scanner to ask the user for the operation type: Scanner op = new Scanner(System.in); Why? You can continue using scannerObject.

• I disagree with the advice of rolfl to close the resources in this case. I don't think System.inshould be closed. If you used both scanner objects (scannerObject and op) and retrieved a value with op after having closed scannerObject you would end up with an exception!