# Order of operations algorithm for calculator

I wrote a simple calculator which uses orders of operations. I would like to know, do you see any serious flaws in logic and what do you think about this solution? It is my second approach to a problem, and the code passes tests (with integers and decimals) for basic operations: ^,|(i used this sign for root square), *, /, +, -.



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

public class OrderOfOperations {
ArrayList<String> contents;
String item;
OrderOfOperations check;

public static void main (String[] args){
Scanner input = new Scanner(System.in);
System.out.println("Enter an operation: ");
String a = input.nextLine();
OrderOfOperations go = new OrderOfOperations();
a = go.brackets(a);
System.out.println("Result: "+a);
}
public String brackets(String s){             //method which deal with brackets separately
check = new OrderOfOperations();
while(s.contains(Character.toString('('))||s.contains(Character.toString(')'))){
for(int o=0; o<s.length();o++){
try{                                                        //i there is not sign
if((s.charAt(o)==')' || Character.isDigit(s.charAt(o))) //between separate brackets
&& s.charAt(o+1)=='('){                         //or number and bracket,
s=s.substring(0,o+1)+"*"+(s.substring(o+1));        //it treat it as
}                                                       //a multiplication
}catch (Exception ignored){}                                //ignore out of range ex
if(s.charAt(o)==')'){                                  //search for a closing bracket
for(int i=o; i>=0;i--){
if(s.charAt(i)=='('){                          //search for a opening bracket
String in = s.substring(i+1,o);
in = check.recognize(in);
s=s.substring(0,i)+in+s.substring(o+1);
i=o=0;
}
}
}
}
if(s.contains(Character.toString('('))||s.contains(Character.toString(')'))||
s.contains(Character.toString('('))||s.contains(Character.toString(')'))){
System.out.println("Error: incorrect brackets placement");
return "Error: incorrect brackets placement";
}
}
s=check.recognize(s);
return s;
}
public String recognize(String s){              //method divide String on numbers and operators
PutIt putIt = new PutIt();
contents = new ArrayList<String>();         //holds numbers and operators
item = "";
for(int i=s.length()-1;i>=0;i--){           //is scan String from right to left,
if(Character.isDigit(s.charAt(i))){     //Strings are added to list, if scan finds
item=s.charAt(i)+item;              //a operator, or beginning of String
if(i==0){
putIt.put();
}
}else{
if(s.charAt(i)=='.'){
item=s.charAt(i)+item;
}else if(s.charAt(i)=='-' && (i==0 || (!Character.isDigit(s.charAt(i-1))))){
item=s.charAt(i)+item;          //this part should recognize
putIt.put();                    //negative numbers
}else{
putIt.put();                //it add already formed number and
item+=s.charAt(i);          //operators to list
putIt.put();                //as separate Strings
if(s.charAt(i)=='|'){       //add empty String to list, before "|" sign,
item+=" ";          //to avoid removing of any meaningful String
putIt.put();        //in last part of result method
}
}
}
}
contents = putIt.result(contents, "^", "|");    //check Strings
contents = putIt.result(contents, "*", "/");    //for chosen
contents = putIt.result(contents, "+", "-");    //operators
return contents.get(0);
}
public class PutIt{
public void put(){
if(!item.equals("")){
item="";
}
}
public ArrayList<String>result(ArrayList<String> arrayList, String op1, String op2){
int scale = 10;                              //controls BigDecimal decimal point accuracy
BigDecimal result = new BigDecimal(0);
for(int c = 0; c<arrayList.size();c++){
if(arrayList.get(c).equals(op1)|| arrayList.get(c).equals(op2)){
if(arrayList.get(c).equals("^")){
result = new BigDecimal(arrayList.get(c-1)).pow(Integer.parseInt(arrayList.get(c+1)));
}else if(arrayList.get(c).equals("|")){
result = new BigDecimal(Math.sqrt(Double.parseDouble(arrayList.get(c+1))));
}else if(arrayList.get(c).equals("*")){
result = new BigDecimal(arrayList.get(c-1)).multiply
(new BigDecimal(arrayList.get(c+1)));
}else if(arrayList.get(c).equals("/")){
result = new BigDecimal(arrayList.get(c-1)).divide
(new BigDecimal(arrayList.get(c+1)),scale,BigDecimal.ROUND_DOWN);
}else if(arrayList.get(c).equals("+")){
result = new BigDecimal(arrayList.get(c-1)).add(new BigDecimal(arrayList.get(c+1)));
}else if(arrayList.get(c).equals("-")){
result = new BigDecimal(arrayList.get(c-1)).subtract(new BigDecimal(arrayList.get(c+1)));
}
try{       //in a case of to "out of range" ex
arrayList.set(c, (result.setScale(scale, RoundingMode.HALF_DOWN).
stripTrailingZeros().toPlainString()));
arrayList.remove(c + 1);            //it replace the operator with result
arrayList.remove(c-1);              //and remove used numbers from list
}catch (Exception ignored){}
}else{
continue;
}
c=0;                     //loop reset, as arrayList changed size
}
return arrayList;
}
}
}


I tested code for inputs such as:

• 1-(1+1)+1,
• 2*(5*(8/2)),
• |3+3.6589,
• 9-5/(8-3)*2+6,
• -1--1--1--1+|4^2,
• 1+7/3*(34.67/23-(-2--5)+6^2),
• 2(2(2(2(2(2))))),
• could you give us a couple of example inputs?
– tim
Mar 4, 2015 at 19:35
• ok, I got it now. 5+(3*2) would be one example, and spaces are not allowed.
– tim
Mar 4, 2015 at 19:44

OOP

Your internal PutIt class doesn't have any state itself, but uses the fields of the enclosing class, which is quite confusing. PutIt could just as well be removed and its methods made part of OrderOfOperations.

It's also rarely needed that a class has a field storing an instance of the class itself (linked lists would be an example where this makes sense). If I remove the check field from your code and call all the methods directly, it still seems to work perfectly.

I don't want to do a complete redesign of your code, but if you do want to use classes, classes such as Operation and Equation might be more useful.

Error Handling

Your code doesn't handle invalid input very well. As I had no idea what valid input looks like, I tried a couple of things:

• 5 + 3 -> java.lang.NumberFormatException: I would expect at least a message of what string could not be formated (although your program should be able to handle spaces and not throw an error at all).
• + 4 2 -> java.lang.ArrayIndexOutOfBoundsException: -1: Again, catching this and presenting a meaningful error message would be nice.

Naming

Variable names should help a reader understand your code. They should be as expressive as possible.

• Yours are often very generic. contents of what? what item? check what? PutIt what's it? and put it where? go where?
• short variable names are also almost never good. c, a, and s are all not very expressive. Especially bad is o as a loop variable, because it results in code like i=o=0, which is quite hard to read.
• methods should be named after what they do. brackets eg doesn't tell me that at all (does it process brackets? does it find brackets?). Same with recognize (what does it recognize? and what does recognize even mean in this context?), put (put what where?) and result (does it print the result? does it create the result? the result of what?).

Misc

• don't import *, but the concrete classes that you need.
• use private or public for fields.
• use a lot more spaces to increase readability (you can use an IDE to format this for you).
• don't ignore exceptions. If you don't want to deal with them, throw them upwards. Just swallowing them will make it very hard to find bugs.
• declare fields in as small a scope as possible. contents and item are both only used in recognize and put. It would be a lot better to declare them inside recognize and pass them as arguments to put.