Math expression solver

I've recently stumbled upon an interesting challenge for me:

You should develop C application, that solves math expressions. Operations such as (+,-,*,/) should be supported, as well as (cos, sin, exp). Application should replace constants Pi and E with built-in values. We may have any amount of spaces between operators and braces.

Example:

stdin: 11 + (exp(2.010635 + sin(PI/2)*3) + 50) / 2

stdout: 111.00000 (may have insignificant deviations)

I am interested in solving this problem using Java. I know that the best way is to use lexical analysis or regexp, but for now it is too complex for me. I am trying to solve this task without them. My code works well, but it looks awful, I think.

My code is also on pastebin.

Can you help me make my code style better?

```
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.util.ArrayList;
import java.util.Arrays;

/**
* Created by Vladimir on 20.02.14.
*/
public class Main {
private static final ArrayList DIVIDERS = new ArrayList
(Arrays.asList('*', '/', '-', '+'));
private static final int RIGHT_DIRECTION = 1;
private static final int LEFT_DIRECTION = -1;

public static void main(String[] args) {
BufferedReader reader = new BufferedReader(new InputStreamReader(System.in));
String expression = "";

System.out.print("Enter expression: ");
try {
expression = reader.readLine();
} catch (IOException e) {
e.printStackTrace();
}
System.out.println("Expression: " + expression);

expression = prepareExpression(expression);

System.out.println("Prepared expression: " + expression);

System.out.println("Answer: " + calc(expression));

}

//Recursive function with the state machine
//states "(", "sin", "cos", "exp", "*", "/", "+", "-"
private static String calc(String expression) {
int pos = 0;
System.out.pr

I agree with that a different approach would be better, so just a few random notes, mostly about the current code:

-
It's good to know that it's already in the JDK, in form a script engine. So, if you don't want to reinvent the wheel just use it:

import javax.script.ScriptEngine;
import javax.script.ScriptEngineManager;

final ScriptEngineManager engineManager = new ScriptEngineManager();
final ScriptEngine engine = engineManager.getEngineByName("JavaScript");
String expression = "11 + (Math.exp(2.010635 + Math.sin(Math.PI/2)*3) + 50) / 2";
System.out.println(engine.eval(expression)); // prints 110.99997794278411

The syntax is not the same (you need the Math. prefix) but I guess it can be changed.

-
Instead of doubles (and floating point numbers) consider using BigDecimals. Floating point numbers are not precise. Here is an example:

Enter expression: 1.03-0.42
Expression: 1.03-0.42
Prepared expression: 1.03-0.42
Solving expression: 1.03-0.42
Solving expression: 0.6100000000000001
Answer: 0.6100000000000001

(It might not fit well for trigonometric functions.)

• Why not use Double or Float to represent currency?

• Effective Java, 2nd Edition, Item 48: Avoid float and double if exact answers are required

-
DIVIDERS currently mutable. You could wrap it with Collections.unmodifiableList to avoid accidental modification. A more compact form is Guava's ImmutableList:

private static final List DIVIDERS = ImmutableList.of('*', '/', '-', '+');

-
In some places I would use aString.contains instead of indexOf. It's more meaningful, closer to the English language, easier to read since you cat get rid of the comparison. In conditionals like this

expression.indexOf("*") > 0 | expression.indexOf("/") > 0

I'd use the even more compact

StringUtils.containsAny(expression, "*/")

(from Apache Commons Lang)

-
The following lines also could be replaced with StringUtils:

int multPos = expression.indexOf("*");
int divPos = expression.indexOf("/");

pos = Math.min(multPos, divPos);
if (multPos < 0)
    pos = divPos;
else if (divPos < 0)
    pos = multPos;

Use indexOfAny:

pos = StringUtils.indexOfAny(expression, "*/")

It's the same.

-
Instead of returning with an error string, like

return "Failure!";

I think you should throw an exception and stop the processing immediately, there is no point to "continue" the processing with wrong subresult (or to pretend that it continues). (The Pragmatic Programmer: From Journeyman to Master by Andrew Hunt and David Thomas: Dead Programs Tell No Lies.)

The same is true for the default branch of calcShortExpr. If you call it with an invalid "divider" it's definitely an error in the program and it should stop.

-
The divider variable actually contains the operator which is a little bit confusing. I'd rename it for better readability.

StackExchange Code Review Q#42323, answer score: 8