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

Simple Calculator - revised

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

Problem

This is my revised code of the Calculator:

```
import java.math.*;
import java.util.*;

public class Calculator {

public static void main(String[] args) {
Scanner sc = new Scanner(System.in);
System.out.println("Welcome to the Calculator!");
do {
System.out.println("\nType a calculation:");
String s = sc.nextLine();
try {
System.out.println("\nThe answer is: " + calculate(s));
} catch (IllegalArgumentException e) {
System.out.println("Oops! Illegal calculation.");
}
} while (doAgain(sc));
System.out.println("\nThank you for using the calculator!");
sc.close();
}

private static boolean doAgain(Scanner sc) {
while (true) {
System.out.println("\nAgain?");
String s = sc.nextLine();
if (s.equalsIgnoreCase("y") || s.equalsIgnoreCase("yes")
|| s.equalsIgnoreCase("t") || s.equalsIgnoreCase("true"))
return true;
else if (s.equalsIgnoreCase("n") || s.equalsIgnoreCase("no")
|| s.equalsIgnoreCase("f") || s.equalsIgnoreCase("false"))
return false;
}
}

public static Double calculate(String s) throws IllegalArgumentException {
IllegalArgumentException exception = new IllegalArgumentException(
"Illegal Calculation.");
ArrayList nums = new ArrayList(s.length());
ArrayList op = new ArrayList(s.length());
getNumsAndOp(nums, op, s, exception);
for (int i = 0; i nums,
ArrayList op, String plusOrMinus) {
int index = op.indexOf(plusOrMinus);
if (plusOrMinus.equals("+")) {
nums.set(index, nums.get(index) + nums.get(index + 1));
} else {
nums.set(index, nums.get(index) - nums.get(index + 1));
}
nums.remove(index + 1);
op.remove(index);
}

private s

Solution

From your previous question:

I have a simple calculator program that needs some reviewing. Is this as efficient as it can get, or is there a better way?

I'll try tackling both processing speed and memory use.

Efficiency issues

Creating new strings often. Examples are on lines 116, 124, 127, 131, 149, 158. Use String.substring when you do need a part of your original string.

Testing boundaries with IndexOutOfBoundsException at 135 and 163. A quick profiling run with VisualVM had your code spending about 80% of its time throwing and catching exceptions in valid expressions. Do an explicit boundary check like i

  • Build a precedence tree. Here you check syntax and perhaps illegal operations (division by zero).



  • Actual evaluation.



This will allow you to handle more or different operators, such as parentheses, negation, and so on.

Look into the
Scanner class for some helpful functions, like nextBigDecimal().

Premature optimisalisation is the root of evil. First get it working, then profile your application to catch efficiency problems, and then see what you can do about any issues that ping on the radar.

Aim for bite-sized functions.
getNumsAndOp is a bit of a beast. multiplyOrDivide and plusOrMinus` feel just about right in size. This will also make profiling easier for both you and the HotSpot compiler.

Code Snippets

throw new IllegalArgumentException("Number expected, found: " + s);

Context

StackExchange Code Review Q#57331, answer score: 4

Revisions (0)

No revisions yet.