# Refactoring calculator expression

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

For example:

(((2*2)+(3*5/5)+(2*5))) outputs 17

((2*2)(3*5/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<String> 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 = "(((2*2)+(3*5/5)+(2*5)))";//17
String testExpression = "((2*2)(3*5/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 + "==" + endIndex); } if (startIndex < endIndex) { double value = calculate(startIndex, endIndex); calculatedValue = value; System.out.println("CalculatedValue=" + value); // Reset arrayList if (startIndex >= 0 || endIndex <= length) { // Resize the list ArrayList<String> tempList = new ArrayList<>(); int j = 0; for (int i = 0; i < startIndex - 1; i++) { tempList.add(data.get(i)); } tempList.add(value + ""); for (int i = endIndex; i < length; i++) { tempList.add(data.get(i)); } data = tempList; process(); } } return calculatedValue; } private void getCalIndex() { endIndex = 0; startIndex = 0; for (String element : data) { endIndex++; startIndex++; if (element.equals(")")) { for (int i = endIndex - 1; i >= 0; i--) { if (data.get(i).equals("(")) { return; } startIndex--; } } } } public double calculate(int startIndex, int endIndex) { int j = 0; element = new Element(); for (int i = startIndex; i < endIndex; i++) { switch (data.get(i)) { case "*" : addOrManipulateEntitySecond('*', i, j); break; case "/" : addOrManipulateEntitySecond('/', i, j); break; case "-" : addOrManipulateEntityFirst('-', i, j); break; case "+" : addOrManipulateEntityFirst('+', i, j); break; default : break; } } /** Printing the Elements **/ double operateCalculate = operate(element.getFirst(), element.getOperator(), element.getSecond()); System.out.println("OperateCacluate=" + startIndex + "=" + endIndex + "=" + operateCalculate); return operateCalculate; } private static void addOrManipulateEntityFirst(char o, int i, int j) { if (element.getOperator() == '\u0000') { element = new Element(Double.valueOf(data.get(i - 1)), Double.valueOf(data.get(i + 1)), o); } else { element.setFirst(element.operate()); element.setSecond(Double.valueOf(data.get(i + 1))); element.setOperator(o); } } private void addOrManipulateEntitySecond(char o, int i, int j) { if (element.getOperator() == '\u0000') { element = new Element(Double.valueOf(data.get(i - 1)), Double.valueOf(data.get(i + 1)), o); } else { element.setSecond(operate(element.getSecond(), o, Double.valueOf(data.get(i + 1)))); // elements[j].setFirst(Double.valueOf(data.get(i+1))); // elements[j].setOperator(o); } } private double operate(double first, char o, double second) { double result = 0; switch (o) { case '*' : result = first * second; break; case '/' : result = first / second; break; case '+' : result = first + second; break; case '-' : result = first - second; break; default : break; } if (result == 0) { result = calculatedValue; } return result; } } public class Element { private double first; private double second; private char operator; private double result; public Element() { } public Element(double first, double second, char operator) { this.first = first; this.second = second; this.operator = operator; System.out.println(this); } public double operate() { switch (operator) { case '*' : result = first * second; break; case '/' : result = first / second; break; case '+' : result = first + second; break; default : break; } return result; } @Override public String toString() { // TODO Auto-generated method stub return "[ " + first + " " + operator + " " + second + " ]"; } public double getFirst() { return first; } public void setFirst(double first) { System.out.println("First : " + first); this.first = first; } public double getSecond() { return second; } public void setSecond(double second) { this.second = second; } public char getOperator() { return operator; } public void setOperator(char operator) { this.operator = operator; } public double getResult() { return result; } public void setResult(double result) { this.result = result; } } import java.util.ArrayList; import java.util.regex.Matcher; import java.util.regex.Pattern; public class CalculatorUtil { public static final String INTEGER_FROM_STRING = "\\d+"; public static final String FLOAT_FROM_STRING = "(\\d+[.]\\d+)"; public static final String OPERATORS_FROM_STRING = "(\\/)|(\\-)|(\\+)|(\\*)|(\\%)|(\\()|(\$$)|(\\^)";
public static final String NUMBERS_FROM_STRING = FLOAT_FROM_STRING+"|"+INTEGER_FROM_STRING;
public static final String NUMBERS_AND_OPERATORS_FROM_STRING = NUMBERS_FROM_STRING+"|"+OPERATORS_FROM_STRING;
public static final String MULTIPLICATION_COMMON = "\\)\\(";
public static final String MULTIPLICATION_VALID =   ")*(";
private static Pattern pattern;
private static Matcher matcher;

public CalculatorUtil() {
// TODO Auto-generated constructor stub
}

/**
*
* @param input
* @return The data as ArrayList<String>
*/
public static ArrayList<String> parseExpression(String input)
{
pattern = Pattern.compile(NUMBERS_AND_OPERATORS_FROM_STRING);

matcher = pattern.matcher(input);

ArrayList<String> data = new ArrayList<String>();

return data;
}

}

• Do you have any experience with writing parsers? There are less convoluted ways to do what you're doing, such as writing a bog-standard recursive descent parser.
– amon
Jan 23, 2014 at 9:14
• Have you considered converting the input to Reverse Polish Notation? Not having the user do it, just taking the input, convert it to RPN, and then evaluate the RPN. This would allow the user to skip unneeded parentheses. Jan 23, 2014 at 9:36
• do google search for "recursive descent parser for arithmetic expressions". Jan 24, 2014 at 8:36