Very basic calculator using methods

Question

I have made a basic calculator using methods. This is my first attempt at using methods and would like to see if I can improve on this as there is a lot of repeated code.

import java.util.Scanner;


public class Calculator {


    public static void main(String[] args) {

        Scanner kb = new Scanner(System.in);

        System.out.println("Simple Calculator");

        System.out.println("\nHere are your options:");
        System.out.println("\n1. Addition");
        System.out.println("2. Subtraction");
        System.out.println("3. Division");
        System.out.println("4. Multiplication");

        System.out.print("\nWhat would you like to do?: ");
        int choice = kb.nextInt();
        System.out.println();


        if (choice == 1){
            addition();
        }
        else if (choice == 2){
            subtraction();
        }
        else if (choice == 3){
            division();
        }
        else if (choice == 4){
            multiplication();
        }

        System.out.println();
        kb.close();
    }

    public static void addition(){

        int nOne, nTwo;
        Scanner kb = new Scanner(System.in);

        System.out.println("Addition");

        System.out.print("\nFirst Number: ");
        nOne = kb.nextInt();

        System.out.print("\nSecond Number: ");
        nTwo = kb.nextInt();

        kb.close();
        System.out.println("\nSum: " + nOne + " + " + nTwo + " = " + (nOne + nTwo));
    }

    public static void subtraction(){
        int nOne, nTwo;
        Scanner kb = new Scanner(System.in);

        System.out.println("Subtraction");

        System.out.print("\nFirst Number: ");
        nOne = kb.nextInt();

        System.out.print("\nSecond Number: ");
        nTwo = kb.nextInt();

        kb.close();
        System.out.println("\nSum: " + nOne + " - " + nTwo + " = " + (nOne - nTwo));
    }

    public static void division(){
        int nOne, nTwo;
        Scanner kb = new Scanner(System.in);

        System.out.println("Division");

        System.out.print("\nFirst Number: ");
        nOne = kb.nextInt();

        System.out.print("\nSecond Number: ");
        nTwo = kb.nextInt();

        kb.close();
        System.out.println("\nSum: " + nOne + " / " + nTwo + " = " + (nOne / nTwo));
    }

    public static void multiplication(){
        int nOne, nTwo;
        Scanner kb = new Scanner(System.in);

        System.out.println("Multiplication");

        System.out.print("\nFirst Number: ");
        nOne = kb.nextInt();

        System.out.print("\nSecond Number: ");
        nTwo = kb.nextInt();

        kb.close();
        System.out.println("\nSum: " + nOne + " x " + nTwo + " = " + (nOne * nTwo));
    }
}

Answer 1

Nice job! I hope you find the comments below useful as you progress.

1. Variable names: Try to have variable names which describe what the variable actually represents or holds. For example, I have no idea why you named your Scanner "kb".
2. Method names: Method names should always be verb phrases (actions) which describe what the method does. So for example, your addition() method might be better named performAddition() or some such. Even just add() might work, though that might be a bit misleading and lead someone reading your code to believe that all the method does is add two numbers, when really the core functionality is user interface.

3. Redundancy: Any time you are copy-pasting code, alarm bells should go off in your head. You do this in multiple places. Most notable is:

   System.out.print("\nFirst Number: ");
   nOne = kb.nextInt();
   
   System.out.print("\nSecond Number: ");
   nTwo = kb.nextInt();
   

What that means is that this is a prime candidate for encapsulation into its own method. Something like this:

private static int[] getNumbers() {
    int[] numbers = new int[2];
    System.out.print("\nFirst Number: ")
    numbers[0] = scanner.nextInt();
    System.out.print("\nSecond Number: ");
    numbers[1] = scanner.nextInt();
    return numbers;
}

Now we can simply call this method whenever we need to get numbers from the user. Much easier, and we've reduced the complexity of our code.

Another thing that's redundant is your constant re-declaration of kb = new Scanner(System.in). You really only need to do this once. If it were me, I'd have it as a static field for the entire class, e.g.,

private static final Scanner STDIN = new Scanner(System.in);

This is the basic rule for how constants are defined in Java. I named the variable STDIN because this is a convention for referring to the "standard input" stream, but SCANNER or something would be just as good.

---

Some other small things

You should consider using a switch statement on your choice variable:

switch(choice) {
case 1:
    performAddition();
    break;
case 2:
    performSubtraction();
    break;
case 3:
    performDivision();
    break;
case 4:
    performMultiplication();
    break;
default:
    System.out.println("Invalid choice!");
    break;
}

... or you could write it all in-line, if it looks better to you, since each block only really does one thing:

switch(choice) {
case 1: performAddition(); break;
case 2: performSubtraction(); break;
// etc.
}

This would also be a perfect use-case for enums, but that my be getting a bit too complicated for a starting project.

---

Small final note: I think you have a bug. The line where you display the results always says "sum", even when you're doing division, multiplication, or subtraction. :)

Answer 2

Your flow is a little clunky.

If I were going to calculate something I would do it in this order:

Number1 then operator then Number2

This way I know what I am doing, especially when playing with division.

I think you should ask for the first number then ask for the operation and then ask for the third number.

Also change your choice to take input of +, -, *, / as well as the numbered choices, in doing this you are one step closer to taking this to a new level with forms or whatever direction your application is going to take away from the console.

Eventually you can add something to this so that it distinguishes the operators separately and the user can write an expression rather than answer questions about the expression, I think that is really the goal right?

This sounds like a fun project!