HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaModerate

Simple Java calculator with two operands

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
simplewithjavatwooperandscalculator

Problem

I am taking a lynda dot com course online to start off. The course asks you to write a simple program:



  • Ask the user to input 2 values



  • Ask the user to input an operation



  • Use the operation on the 2 values and print the result




This was very different from the solution given by the course. Could anyone take a quick look at it and tell me if there's something I'm doing fundamentally wrong or inefficient? My suspicion is that I'm using too much memory.

package com.example.java;

import java.util.Scanner;

public class Main {

    public static void main(String[] args) {

        Scanner scanner = new Scanner(System.in);

        System.out.print("Enter a numeric value: ");
        String input1 = scanner.nextLine();
        System.out.print("Enter a numeric value: ");
        String input2 = scanner.nextLine();

        double double1 = Double.parseDouble(input1);
        double double2 = Double.parseDouble(input2);

        System.out.print("Choose and operation (+ - * /): ");
        String input3 = scanner.nextLine();

        double resultAdd = double1 + double2;
        double resultSub = double1 - double2;
        double resultMul = double1 * double2;
        double resultDiv = double1 / double2;

        switch (input3) {
            case "+":
                System.out.println("The answer is " + resultAdd);
                break;
            case "-":
                System.out.println("The answer is " + resultSub);
                break;
            case "*":
                System.out.println("The answer is " + resultMul);
                break;
            case "/":
                System.out.println("The answer is " + resultDiv);
                break;
        }
    }
}


Here is what the instructor wrote, for the record:

```
package com.example.java;

import java.util.Scanner;

public class Calculator2 {

public static void main(String[] args) {
String s1 = getInput("Enter a numeric value: ");
String s2 = ge

Solution

Your code

Simple is nice. But it would have been good to decompose your solution to multiple methods, for example at least:

  • a method to read a double



  • a method to perform the calculation: take the numbers and an operator as parameter and return the result



Other issues:

  • Don't pre-calculate the result of all possible calculations when you will only use one of them



  • The print statements are repetitive. If you decomposed the solution to multiple methods, naturally you would have factored out duplicated elements.



  • Naming, as @Caridorc already covered.



The instructor's code

Sort of nicely written, decomposed, but this is bad code:

-
Delaying the validation of input by carrying around the String values of supposed numeric values is a terrible idea. As @Caridorc said it too, the numeric input should have been read into the desired types (double in this example) and lift the burden of uncertainty from the rest of the code.

-
The getInput method is a good idea, but poorly executed. Creating a new Scanner for every piece of input is a waste. Just like there is one System.in, a single Scanner could be used and passed to the method. The method is a huge missed opportunity for actually validating the input, and returning the desired type, instead of a String.

-
Large try blocks are also better to avoid, and the large try block in the middle is just another consequence of delaying the input validation, that could have been done much better earlier.

Context

StackExchange Code Review Q#110163, answer score: 12

Revisions (0)

No revisions yet.