patternjavaModerate
Simple Java calculator with two operands
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:
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.
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
- 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:
Other issues:
The instructor's code
Sort of nicely written, decomposed, but this is bad code:
-
Delaying the validation of input by carrying around the
-
The
-
Large
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.