Reverse Polish Notation Calculator in Java

Question

I created a highly effective Reverse Polish Notation calculator using Java 8, but am uncertain if there are any better ways to handle the problem. Feedback on any evident taboos and bugs is greatly appreciated!

public final class RPN {

    /**
     * Removes all characters that are not operators, whitespace, or digits
     *
     * @param expr the expression to parse
     * @return the parsed expression
     */
    private static String parseExpression(String expr) {
        String parsedExpr = expr.replaceAll("[^\\^\\*\\+\\-\\d/\\s]", "");
        String trimmedExpr = parsedExpr.replaceAll("\\s+", " ");
        return trimmedExpr;
    }

    /**
     * Computes the outcome of a given expression in Reverse Polish Notation
     *
     * @param expr the expression to compute
     */
    public static void compute(String expr) throws
            ArithmeticException,
            EmptyStackException {
        String validExpr = parseExpression(expr);
        Stack<Double> stack = new Stack<>();

        System.out.println(validExpr);
        System.out.println("Input\tOperation\tStack after");

        for (String token : validExpr.split("\\s")) {
            System.out.print(token + "\t");

            double secondOperand = 0.0;
            double firstOperand = 0.0;

            switch (token) {
                case "+":
                    System.out.print("Operate\t\t");

                    secondOperand = stack.pop();
                    firstOperand = stack.pop();

                    stack.push(firstOperand + secondOperand);
                    break;
                case "-":
                    System.out.print("Operate\t\t");

                    secondOperand = stack.pop();
                    firstOperand = stack.pop();

                    stack.push(firstOperand - secondOperand);
                    break;
                case "*":
                    System.out.print("Operate\t\t");

                    secondOperand = stack.pop();
                    firstOperand = stack.pop();

                    stack.push(firstOperand * secondOperand);
                    break;
                case "/":
                    System.out.print("Operate\t\t");

                    secondOperand = stack.pop();
                    firstOperand = stack.pop();

                    if (secondOperand == 0.0) {
                        throw new ArithmeticException("Cannot divide by zero!");
                    }

                    stack.push(firstOperand / secondOperand);
                    break;
                case "^":
                    System.out.print("Operate\t\t");

                    secondOperand = stack.pop();
                    firstOperand = stack.pop();

                    stack.push(Math.pow(firstOperand, secondOperand));
                    break;
                default:
                    System.out.print("Push\t\t");
                    stack.push(Double.parseDouble(token));
                    break;
            }

            System.out.println(stack);
        }

        System.out.println("Final Answer: " + stack.pop());
    }

    /**
     * What runs the program
     *
     * @param args the command line arguments
     */
    public static void main(String[] args) {
        try {
            String expr = Arrays.toString(args);
            System.out.println(expr);
            compute(expr);
        } catch (Throwable err) {
            System.out.println(err.getMessage());
        }
    }
}

Answer 1

Your parseExpression() function isn't parsing anything. Rather, it sanitizes the string. Sanitizing input is a rather risky business. For example, any decimal points will be discarded! What if I try to use ! as the factorial operator? That would be silently ignored as well. If you can't handle the input in a reasonable way, it should be an error.

It's odd that you convert args from an array to a string, then split it up again. You could either use args directly, or split just args[0].

Catching Throwable is probably a bad idea. Anything that is not an Exception is generally not meant to be caught and handled.

It's actually not true that you cannot divide by zero. That's an error only when performing integer arithmetic. Why not let Double.POSITIVE_INFINITY be a valid result?

The code would be simpler if you didn't assign to variables firstOperand and secondOperand. I'd write it more like this:

import java.util.*;

public class RPN {

    /**
     * Computes the outcome of a given expression in Reverse Polish Notation
     *
     * @param expr the expression to compute
     */
    public static void compute(String expr) throws
            ArithmeticException,
            EmptyStackException {
        Stack<Double> stack = new Stack<>();

        System.out.println(expr);
        System.out.println("Input\tOperation\tStack after");

        for (String token : expr.split("\\s+")) {
            System.out.print(token + "\t");
            switch (token) {
                case "+":
                    System.out.print("Operate\t\t");
                    stack.push(stack.pop() + stack.pop());
                    break;
                case "-":
                    System.out.print("Operate\t\t");
                    stack.push(-stack.pop() + stack.pop());
                    break;
                case "*":
                    System.out.print("Operate\t\t");
                    stack.push(stack.pop() * stack.pop());
                    break;
                case "/":
                    System.out.print("Operate\t\t");
                    double divisor = stack.pop();
                    stack.push(stack.pop() / divisor);
                    break;
                case "^":
                    System.out.print("Operate\t\t");
                    double exponent = stack.pop();
                    stack.push(Math.pow(stack.pop(), exponent));
                    break;
                default:
                    System.out.print("Push\t\t");
                    stack.push(Double.parseDouble(token));
                    break;
            }

            System.out.println(stack);
        }

        System.out.println("Final Answer: " + stack.pop());
    }

    /**
     * Runs the calculation for the RPN expression in args[0].
     */
    public static void main(String[] args) {
        try {
            compute(args[0]);
        } catch (Exception err) {
            System.out.println(err.getMessage());
        }
    }
}

Sample run:

$ java RPN '3 5 +'
3 5 +
Input   Operation   Stack after
3   Push        [3.0]
5   Push        [3.0, 5.0]
+   Operate     [8.0]
Final Answer: 8.0