Determine if simple arithmetic expression is valid This looks very messy to me, and I'm fairly new to java, so I don't know what data structures could have helped me achieve the same result more efficiently. I just went ahead and brute-forced all possible failing scenarios:

Could this be further optimized to reduce the number of checks?

public static boolean isAnOperator(char c){
        return (c == '*' || c == '/' || c == '+' || c == '-' || c == '%');
    }

    private static boolean isValidExpression(String expression){
        //TEST 1: False if expression starts or ends with an operator
        if (isAnOperator(expression.charAt(0)) || isAnOperator(expression.charAt(expression.length()-1)))
            return false;

        //TEST 2: False if test has mismatching number of opening and closing parantheses
        int openParenthCount = 0;
        int closedParenthCount = 0;
        int consecutiveOpCount = 0;

        for (int i = 0; i &lt; expression.length(); i++){
            if (expression.charAt(i) == '('){
                openParenthCount++;
                consecutiveOpCount = 0;
                //SUBTEST: False if expression ends with '('
                if (i == expression.length()-1) return false;
            }
            if (expression.charAt(i) == ')'){
                closedParenthCount++;
                consecutiveOpCount = 0;
                //SUBTEST: False if expression starts with ')'
                if (i == 1) return false;

            }
            if (isAnOperator(expression.charAt(i))){
                consecutiveOpCount++;
                //TEST 3: False if operator is preceded by opening paranthesis or followed by closing paranthesis
                if (expression.charAt(i-1) == '(' || expression.charAt(i+1) == ')')
                    return false; 
            }
            //TEST 4: False if 2 operators found next to each other
            if (isAnOperator(expression.charAt(i)) || consecutiveOpCount &gt; 0)
                return false; 
        }
        if (openParenthCount != closedParenthCount)
            return false;

        //LAST TEST 5: All preceding checks ensure that
        // by this point, all exisiting parantheses open and close properly
        return true;
    }
It is not always an easy task to solve this sort of problem. For data structures, we traditionally use a stack for open and close parenthesis but this is only needed if the type of parenthesis can differ and need nesting correctly. For only "(" and ")" I simmply use an int.

I re-wrote the isAnOperator to a switch statement - its personal choice that one.

I added a isANumber function to aid some checking.

I unit tested the function with 9 tests and worked till it past them all. (JUnitParams, read it as the string is the expression and the boolean is what I expect your function to return)

Your original code fails 3 of these tests - but it may because of not handling spaces in the expressions but you didn't provide any example data

To handle parenthesis I used a counter - but decrement it if I find a close. If I get into a negative number then something went wrong and if it isn't 0 at the end it was wrong. You can work out from the final value if there were too many close or opens. Simple boolean values keep track of wether the last character was an operation or a open parenthesis.

I welcome feedback on this answer - I'm trying to keep it as close to the original code as possible

$(
            $("()", true),
            $("3", true),
            $("3 + 2", true),
            $("(3 + (2))", true),
            $(")", false),
            $("+3", false),
            $("3+-2", false),
            $("(3 + (3 + 2)", false),
            $("(x + 3)", false)
    );


I've re-written the function to be cleaner with less variables:

public static boolean isAnOperator(char c) {
    switch (c) {
        case '*':
        case '/':
        case '+':
        case '-':
        case '%':
            return true;
        default:
            return false;
    }
}
public static boolean isANumber(char c){
    return ((int)c) &gt;= 48 &amp;&amp; ((int)c) &lt;= 57;
}

public static boolean isValidExpression2(String expression) {
    // TEST 1
    if (isAnOperator(expression.charAt(0)) || isAnOperator(expression.charAt(expression.length() - 1))) {
        return false;
    }

    int openParenthCount = 0;
    boolean lastWasOp = false;
    boolean lastWasOpen = false;

    for (char c : expression.toCharArray()) {
        if(c == ' ') continue;
        if (c == '(') {
            openParenthCount++;
            lastWasOpen = true;
            continue;
        } else if (c == ')') {
            if (openParenthCount &lt;= 0 || lastWasOp) {
                return false;
            }
            openParenthCount--;
        }else if (isAnOperator(c)){
            if (lastWasOp || lastWasOpen) return false;
            lastWasOp = true;
            continue;
        }else if(!isANumber(c)){
            return false;
        }
        lastWasOp = false;
        lastWasOpen = false;
    }
    if(openParenthCount != 0) return false;
    if(lastWasOp || lastWasOpen) return false;
    return true;
}