Order of operations algorithm for calculator

Question

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("")){
 contents.add(0,item);
 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))))),

• tim: could you give us a couple of example inputs?
• tim: ok, I got it now. 5+(3*2) would be one example, and spaces are not allowed.

Answer 1

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.