Simple beginner Java calculator

Question

This is my first calculator in Java. I just started learning a week ago so any feedback (positive and negative) is welcome. Tell me what's good and what I can improve on.

import java.util.Scanner;

public class Calculator {

    public static void main(String[] args) {

        // Creates scanner object
        Scanner scanner = new Scanner(System.in);

        // Controls the start and stop of the program
        boolean run = true;

        // Sets and resets the operand and answer variables
        int fnum = 0;
        int snum = 0;
        int answer = 0;

        // While loop that keeps the program running until user exits
        while (run) {

            // Calculator interface
            System.out.println("=====================================");
            System.out.println("           Calculator V1.0         ");
            System.out.println("=====================================");
            System.out.println("1. Addition");
            System.out.println("2. Subtraction");
            System.out.println("3. Multiplication");
            System.out.println("4. Division");
            System.out.println("5. Exit");
            System.out.printf("Please pick an operation(1-5): ");

            // Prompts user to choose an operation
            int operation = scanner.nextInt();

            // Does stuff depending on what integer the user enters
            switch (operation) {
            case 1:
                System.out.printf("Enter first number: ");
                fnum = scanner.nextInt();
                System.out.printf("Enter second number: ");
                snum = scanner.nextInt();
                answer = fnum + snum;
                System.out.println("The answer is: " + answer);
                break;

            case 2:
                System.out.printf("Enter first number: ");
                fnum = scanner.nextInt();
                System.out.printf("Enter second number: ");
                snum = scanner.nextInt();
                answer = fnum - snum;
                System.out.println("The answer is: " + answer);
                break;

            case 3:
                System.out.printf("Enter first number: ");
                fnum= scanner.nextInt();
                System.out.printf("Enter second number: ");
                snum = scanner.nextInt();
                answer = fnum * snum;
                System.out.println("The answer is: " + answer);
                break;

            case 4:
                System.out.printf("Enter first number: ");
                fnum = scanner.nextInt();
                System.out.printf("Enter second number: ");
                snum = scanner.nextInt();
                answer = fnum / snum;
                System.out.println("The answer is: " + answer);
                break;

            case 5:
                System.out.println("Thank you for using the calculator!");
                scanner.close();
                run = false;
                break;

            default:
                System.out.println("Invalid operation!");
                break;
            }
        }
    }
}

Answer 1

Comments

// Does stuff depending on what integer the user enters

That is not a good comment. A comment should say why the code does something if it is not clear and the code should say what it does (if the code is unclear, you can use a comment, but this is also a signal you should write clearer code), this comment is just plain noise.

If you are using the comment to remember what the code does, not just because some teacher/book said to, a comment here is perfectly fine:

// Choose action based on input

---

Duplicated code

case 1:
    System.out.printf("Enter first number: ");
    fnum = scanner.nextInt();
    System.out.printf("Enter second number: ");
    snum = scanner.nextInt();
    answer = fnum + snum;
    System.out.println("The answer is: " + answer);
    break;

case 2:
    System.out.printf("Enter first number: ");
    fnum = scanner.nextInt();
    System.out.printf("Enter second number: ");
    snum = scanner.nextInt();
    answer = fnum - snum;
    System.out.println("The answer is: " + answer);
    break;

Anything catch your eye there? Lots and lots of copy/pasted code. Most of that can be moved to before or after the switch:

System.out.printf("Enter first number: ");
fnum = scanner.nextInt();
System.out.printf("Enter second number: ");
snum = scanner.nextInt();

switch (operation) {
case 1:
    answer = fnum + snum;
    break;

case 2:
    answer = fnum - snum;
    break;
}

System.out.println("The answer is: " + answer);

Output

This really is a nitpick for now, but this:

System.out.println("           Calculator V1.0         ");

Will display the same as this:

System.out.println("           Calculator V1.0");

Methods

This is a pretty straightforward program, but you should start separating unique components of your program and creating more methods. For example, you could create a method for input, a method for calculating the answer, and a method for output.

Enums

Another thing you should look into is using an enum to designate the selected operation. Using case Operation.Addition: is clearer than case 1:, and there is a very limited set of data that allowed, which is demonstrated and enforced by the enum, which only accepts the allowed set, and is not enforced by an int.

Bug

Your division operation has a bug:

snum = scanner.nextInt();
answer = fnum / snum;

What happens when the user enters 0? Your program will crash. You must always test for division by 0 in cases like this:

snum = scanner.nextInt();
if (snum == 0) { 
    System.out.println("Invalid input - cannot divide by 0.");
    break;
}
answer = fnum / snum;

Answer 2

Comments

Since this is a beginner program you can be excused for the very bad comments, but you still need to hear about them.

Comments like this:

    // Creates scanner object
    Scanner scanner = new Scanner(System.in);

    // Controls the start and stop of the program
    boolean run = true;

    // Sets and resets the operand and answer variables
    int fnum = 0;
    int snum = 0;
    int answer = 0;

    // While loop that keeps the program running until user exits

Are all pretty much useless. Your comments should typically explain Why you have done something, and almost never explain What your code is doing.

It is completely obvious that a new scanner object is being created, and that the while loop will keep the program running until the user exits.

On the other hand, the comment that you have for fnum, snum, and answer is somewhat helpful. Unfortunately it is helpful for the wrong reasons.

Bad Variable Names

These are bad variable names:

int fnum = 0;
int snum = 0;

You should make a strong effort to make your variable names as descriptive as possible. In this case I would recommend simply calling the variables firstNumber and secondNumber. Using abbreviations like this is almost never worth the characters that you will save.

StringBuilder

Repeatedly calling System.out.println is very resource intensive. It doesn't matter very much for this program, but it could matter a lot when you are working on something more complicated later on. On the other hand, StringBuilder is very efficient.

So this:

System.out.println("=====================================");
System.out.println("           Calculator V1.0         ");
System.out.println("=====================================");
System.out.println("1. Addition");
System.out.println("2. Subtraction");
System.out.println("3. Multiplication");
System.out.println("4. Division");
System.out.println("5. Exit");
System.out.printf("Please pick an operation(1-5): ");

Could become this:

StringBuilder stringBuilder = new StringBuilder();
stringBuilder.append("=====================================");
stringBuilder.append("\n");
stringBuilder.append("           Calculator V1.0         ");
stringBuilder.append("\n");
stringBuilder.append("=====================================");
stringBuilder.append("\n");
System.out.println(stringBuilder.toString());

It's more lines of code but it is much more performant.

EDIT As noted in the comments, in this case it is better to use a constant instead of StringBuilder. However if you are ever building a string inside of a loop, you will definitely want to be familiar with StringBuilder because in that case it really is much better.

As for the constant, you have a couple of options. At the very least you could make this a constant rather than typing it out twice:

String DIVIDING_LINE = "=====================================";

Note that constants in Java are typically declared in all caps. Or you could make the whole thing a constant like this:

    String MENU = "=====================================\n"
                + "           Calculator V1.0\n"
                + "=====================================\n"
                + "1. Addition\n"
                + "2. Subtraction\n";

That way if you have to create the menu again, you don't have to build the whole string again, you can just use the constant.

Answer 3

        // Controls the start and stop of the program
        boolean run = true;

        // Sets and resets the operand and answer variables
        int fnum = 0;
        int snum = 0;
        int answer = 0;

        // While loop that keeps the program running until user exits
        while (run) {

You can do without the run variable if you change the break in the switch to a return.

The comment before the int variables is currently incorrect, but we can make it correct by moving them into the loop.

        // While loop that keeps the program running until user exits
        while (true) {
            // Sets and resets the operand and answer variables
            int first = 0;
            int second = 0;
            Integer answer = null;

I'm not convinced that you need num in the name. You can always see that they are numbers by looking at the int in the declaration. The f and s aren't that clear about what they are intended to represent, so I'd expand them. Another possibility would be firstOperand and secondOperand. Operand is a more precise name than number and indicates that .

Consider defining at least one method to shorten the code in main.

Incorporating all suggestions (including some I like from other answers):

public class Calculator {
    public static String getMenu() {
        StringBuilder builder = new StringBuilder();

        builder.append("=====================================\n");
        builder.append("           Calculator V1.0\n");
        builder.append("=====================================\n");
        builder.append("1. Addition\n");
        builder.append("2. Subtraction\n");
        builder.append("3. Multiplication\n");
        builder.append("4. Division\n");
        builder.append("5. Exit\n");
        builder.append("Please pick an operation(1-5):  ");

        return builder.toString();
    }

    public static void displayAnswer(int operation, int first, int second) {
            int answer;
            switch (operation) {
                case 1:
                    answer = first + second;
                    break;
                case 2:
                    answer = first - second;
                    break;
                case 3:
                    answer = first * second;
                    break;
                case 4:
                    if (0 == second) {
                        System.out.println("Invalid input - cannot divide by 0.");
                        return;
                    }
                    answer = first / second;
                    break;
                default:
                    System.out.println("Invalid operation!");
                    return;
            }

            System.out.println("The answer is: " + answer);
    }

    public static void main(String[] args) {
        Scanner scanner = new Scanner(System.in);
        String menu = getMenu();

        while (true) {
            System.out.print(menu);

            int operation = scanner.nextInt();

            if (5 == operation) {
                System.out.println("Thank you for using the calculator!");
                scanner.close();
                return;
            }

            System.out.print("Enter first number: ");
            int first = scanner.nextInt();
            System.out.print("Enter second number: ");
            int second = scanner.nextInt();

            displayAnswer(operation, first, second);
        }
    }
}

I'm not sure one way or the other on whether to use StringBuilder over System.out.println. If you do use it, save the string so you only need to build it once.

You don't need to put a break; in the last case of the switch statement. It doesn't hurt anything to have it, but it is unnecessary. Personally, I normally include it for a named case and leave it off for a default case. I feel that this increases the difference between default: and other cases.

You can use the labeled continue to avoid other special handling in the invalid input case.

It's possible to argue that it would be better to split displayAnswer into two methods where one calculates the answer and the other handles the display. That would complicate the logic here. It would help more if the calculator had buttons, etc. that needed to be handled. Because then you wouldn't be trying to handle operations, an exit signal, and invalid input all in the same block of code.

Answer 4

There's some answers above dealing with optimisations that you should only need to care much further down the line (that's not saying that there is some truth to their comments).

I think the most important thing I would change about your code is the use of a Class structure, to define a calculator Object. This is an object you can call methods on to perform the actions, much in the same way as a traditional calculator. This has many benefits, but for now it's just "object oriented" design. Currently your Calculator class is just there to hold a "main()" method.

Note too the use of BigDecimal as a basis for the calculation. A "float" or "double" would also work to a point, but because of the way a double is stored, it can't accurately store certain values (0.1 for example). One thing to note with BigDecimal is the that class is immutable, that is once it has been created with a value, it will always contain this value (the same as the String class). This means you need to assign the result of an operation to a variable. See the "add" method below.

    public class Calculator {
        private BigDecimal currentValue;

        public Calculator() {
            clearAll();
        }

        public void clearAll() {
            this.currentValue = BigDecimal.ZERO;
        }

        public BigDecimal currentValue() {
            return this.currentValue();
        }

        public void add(BigDecimal value) {
            this.currentValue = currentValue.add(value);
        }

        // these methods I'll let you implement
        public void subtract(BigDecimal value)
        public void multiply(BigDecimal value)
        public void divide(BigDecimal value)
    }

To use the Calculator in your "main" method, you would create a new instance of the Calculator, as a simple example:

public static void main(String[] args) {

    Calculator calculator = new Calculator(); 

    String entry = "123.999";

    calculator.add(new BigDecimal(entry));

To print a BigDecimal out in the console as a String, you may need to use a DecimalFormat to print it in normal (not scientific) notation.

...
BigDecimal result = calculator.getCurrentValue();

// DecimalFormat with 10 decimal places
DecimalFormat outputFormat = new DecimalFormat("#0.##########"); 

String answer = outputFormat.format(result);
System.out.println("The answer is: " + answer);
...