Optimizing "simplifier"

Question

Basically, I'm trying to make a Simplifier that can take inputs in the form of a String and print out the solution step-by-step. Now that I've got it working, I think it has some cleanliness issues.

public static void test(String eqn) {
    char[] order = { '^', '*', '/', '+'};   //order of precendece
    int found, i, j;
    eqn = eqn.replace("-", "+-");       //Subtracting = Adding (-)ve number.
    eqn = eqn.replace(" ", "");         //Spaces can cause a lot of trouble.
    String eqnC, left, right;
    System.out.println("=>\t"+eqn);
    for(char op : order) {
        while ((found = eqn.indexOf(op)) > -1) {
            left = eqn.substring(0, found);
            right = eqn.substring(found + 1);
            //keep 'walking' up and down the String till a non-number encoutered.
            for( i = left.length() - 1; i >= 0; i--)
                if(!isPONumber(left.charAt(i)))
                    break;
            for( j = 0; j < right.length(); j++)
                if(!isPONumber(right.charAt(j)))
                    break;
            double lV = Double.parseDouble(left.substring(++i));    //left operand
            double rV = Double.parseDouble(right.substring(0, j));  //right operand
            eqn = left.substring(0, i) + putV(lV, rV, op) + right.substring(j);
            System.out.println("=\t" + eqn);
        }
    }
}

private static String putV(double a, double b, char op) {
    switch(op) {
        case '^':   return Math.pow(a, b) + "";
        case '/':   return (a / b) + "";
        case '*':   return (a * b) + "";
        case '+':   return (a + b) + "";
    }
    return null;
}

/*
 * Limit (for now) : decimal support.
 */
private static boolean isPONumber(char c) {
    return Character.isDigit(c) || c == '.' || c == '-';
}

Here are some outputs:

=>  2*84/6+7^2
=   2*84/6+49.0
=   168.0/6+49.0
=   28.0+49.0
=   77.0

=>  3^3/3^2
=   27.0/3^2
=   27.0/9.0
=   3.0

=>  400^0.5
=   20.0

I'd be grateful if anyone would comment and criticize and help me to clean-up my code. I also invite suggestions for making it better, improvising it and so on.

Answer 1

I love the replacement of - with +-. It's simple and effectful. I'd like to see a similar use of * and / by introducing an operator for the reciprocal, e.g. ~. This would of course require using a custom function for parsing the numbers instead of parseDouble (or the introduction of real unary operators), so that may cancel out any advantages.

EDIT: Just like a - b is equal to a + -b, a / b is equal a * 1/b. 1/b is called the reciprocal of b. You could define an operator (such as ~ or any other character, that you aren't using) to represent the reciprocal, and replace / with *~, so that 4/2 becomes 4*~2. That requires you to add ~ as a valid character to isPONumber, and replace parseDoublesince it doesn't know what ~ is (I just invented it). See next edit.

I'm not really a fan of using parseDouble directly anyway. I'd at least replace it with a custom function that possibly uses parseDouble internally.

EDIT: By using parseDouble you are saying "my programm uses the same syntax for number literals as Java". That is not a problem, until you realize that the Java syntax may not support every thing you need, for example because you just decided to support the reciprocal operator, or you want the program to be international and some countries use a comma instead of a period for the decimal mark, e.g. 1,2 instead of 1.2, but parseDouble doesn't support this. And instead of needing to search your whole code and replace each occurance of parseDouble, you should have one function

static public double parseNumber(String number) {
  return Double.parseDouble(number);
}

Now if you need to change the syntax of your numbers, then you only have one defined place where you need to do it. For example to support the reciprocal:

static public double parseNumber(String number) {
  if (number.charAt(0) == '~') {
    return 1 / parseNumber(number.substring(1));
  }
  return Double.parseDouble(number);
}

(END EDIT)

I also don't like the use of the variable names i and j here, since they are used outside the for loop. Something like leftOffset/rightOffset may be more appropriate.

You could drop the variables left and right and search the original string with i and j starting at found/found + 1.

The use of a enum for the operators would also be useful. Something like:

interface DoubleOperation {
  public double calc(double a, double b);
}

enum Operator {
  POWER("^", new DoubleOperation() { public double calc(double a, double b) { return Math.pow(a, b); } }),
  DIVISION("/", new DoubleOperation() { public double calc(double a, double b) { return a / b; } });
  // ...

  public String op; // Use private and a getter in real code
  public DoubleOperation calc;

  Operator(String op, DoubleOperation calc) {
    this.op = op;
    this.calc = calc;
  }
}

(Look into the just released JDK 8 for lamba expressions to simplify this.)

Answer 2

Algorithm

The algorithm works well enough. You have captured the essence of the problem in your code, and reading your code it is apparent what it does, and why.

This is a good thing. It is not very often that the intent of the code is so easy to discern.

The draw-back of your algorithm is that it is not going to be able to support the more esoteric operations like parenthesis e.g. ( 123 - 45 ) ^ 2 .... That sort of support will require a preprocessing step.

Keeping the equation as a string is convenient for some things, but i would have preferred to see the process broken down in to components, for example, a parse step could break the input down to List<String> eqnParts = parse(eqn). at the same time, it could be trimming the white-space. Then, you just need to scan the List<String> and find 3 items separated by the highest precedence operator, and swap all three with the resulting value.

Code Style

You should read through the Code Style Guidelines for Java.

This code, for example, has a number of faults:

        for( i = left.length() - 1; i >= 0; i--)
            if(!isPONumber(left.charAt(i)))
                break;
        for( j = 0; j < right.length(); j++)
            if(!isPONumber(right.charAt(j)))
                break;

It should rather look something like:

        for (int i = left.length() - 1; i >= 0; i--) {
            if(!isPONumber(left.charAt(i))) {
                break;
            }
        }
        for (int j = 0; j < right.length(); j++) {
            if(!isPONumber(right.charAt(j))) {
                break;
            }
        }

Notes:

1. declare int i and int j inside the loop.
2. space after the keyword for.
3. braces even for '1-liner' conditional statements.

Double.parseDouble

Double.parseDouble() throws the unchecked exception NumberFormatException.

The exception thrown does not give great deals of information about why the number is not valid.

I always recommend wrapping Double.parseDouble (and Integer.parseInteger(), etc.) in a try-catch that reports what value was parsed, as well as the exception that was thrown. Otherwise debugging is a pain.

I often have a helper function like:

public static final double parseDouble(String value) {
    try {
        return Double.parseDouble(value);
    } catch (NumberFormatException nfe) {
        throw new IllegalArgumentException("Unable to Double.parseDouble(" + value + ")", nfe);
    }
}