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

Refactoring calculator expression

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

Problem

I was doing a simple exercise to calculate the amount based on an expression.

For example:

(((22)+(35/5)+(2*5))) outputs 17

((22)(35/5)(2*5)) output 120 etc

I have written the code, but I am not satisfied with what I've done. Could you please help me to review the code and suggest the best approach for that?

```
import java.util.ArrayList;

public class Calculator {

private static ArrayList data;

private static Element element = new Element();
int startIndex = 0;
int endIndex = 0;
public int length = 0;
boolean startFlag = false;
public double calculatedValue = 0;
public Calculator() {
// TODO Auto-generated constructor stub
}

public static void main(String[] args) {
// String testExpression = "(((22)+(35/5)+(2*5)))";//17
String testExpression = "((22)(35/5)(2*5))"; // 120--

new Calculator().parsedExpression(testExpression);
}

public void parsedExpression(String testExpression) {
System.out.println(testExpression + " Before formatting");
testExpression = testExpression.replaceAll("\\)\\(", ")*("); // To
// change
// (a+b)(b+c)
// to
// (a+b)*(b+c)
System.out.println(testExpression + " After first formatting");
data = CalculatorUtil.parseExpression(testExpression);
System.out.println("Total Sum====" + process());

}
public double process() {
System.out.println("Array:ist" + data);
startIndex = 0;
endIndex = 0;
length = data.size();
System.out.println("===" + data.indexOf("("));
if (data.indexOf("(") >= 0) {
getCalIndex();
System.out.println("startIndex=" + startIndex

Solution

I agree with the point in the comments. The approach you can implement that would simplify your code heavily is a recursion. The structure would be following the idea that you have interface/class Element, lets say, as you have it here but that would be just an interface. This interface would have implementord for binary and unary and 0-nary operation. Binary op would have childs plus,minus and so on, unary would have childs parentheses and unary minus and 0-nary's child is a number. Number has zero arguments so hence it can be considered 0-nary operation. The childs of those classes would be of type Element.

The interface would implement the method calculate and if you want to print it out also a method print(outputstream).

That is regarding how to parse. You would definitely like to use recursion because it is easier and shorter to write and also the case you are solving is of a recursive character.

I liked how you used matching for checking if it a valid input. That is always good to validate your inputs. With the recursive approach you may validate it on the go, not all at the beginning.

Context

StackExchange Code Review Q#39863, answer score: 3

Revisions (0)

No revisions yet.