Four-function expression evaluator

I've been making an interpreter for my own programming language that I've been working on as a hobby. I made an expression evaluator that can evaluate simple mathematical expressions.

It understands +, -, /, *, brackets and the order of operations. I haven't put a modulus operator in the evaluator because I'm going to make a function for modulus instead.

I works in all of the tests I've tried. But I'd appreciate of anyone knows of any complicated expressions that it fails to evaluate.

It also understands that a positive number of minuses is that same as a plus.

It evaluates expressions like this:

(10 + (20 * - (5 + 9)))

and produce the correct output of, -270.

I'd just like to know whether there are any ways I can improve my expression evaluator and whether there are any complicated expressions that don't evaluate correctly.

Also, is there a better way than using the time command on OSX to find out how fast it evaluates because the time command only does to 3 decimal places so it's not accurate enough?

My expression evaluator evaluates the above expression like this:

(10 + (20 * - (14)))

-
It first does the deepest brackets.

(10 + (20 * -14))

-
Then, since there's a minus it multiplies the result of the brackets by -1.

(10 + (-280))

-
Then it does the multiply in the brackets.

(-270)

-
Then it does the addition with the 10 and the -280.

```
#include
#include
#include
#include
#include
#include
#include
#include
#include

using namespace std;

vector rdo_num_stack;
int opp_count = 0;

bool rdo_ws(char c) {
if (c != ' ')
return 0;
else
return 1;
}

char rdo_expr_item_type(char c) {

switch(c) {
case '0':
case '1':
case '2':
case '3':
case '4':
case '5':
case '6':
case '7':
case '8':
case '9':
case '.':
c = 'n';
break;
case '+':
case '-':

Loki Astari already covered a number of good points, which I will not repeat.

Algorithms and data structures

The standard library contains a number of ready-made algorithms and data structures that can make your code easier to read and understand.

For example, you have these lines:

opp_count = 0; // line 393, new lines removed for brevity
for (i = 0; i < rdo_num_stack.size();i++) {        
    if (rdo_num_stack[i] == "+" or rdo_num_stack[i] == "-" or rdo_num_stack[i] == "*" or rdo_num_stack[i] == "/") {
        opp_count++;
    }
}

Instead, you could define a set containing your operator symbols and a function, and then use count_if() included from the ` header:

// At top level
static const std::set operators = {"+", "-", "/", "*"};
bool is_operator(const std::string& s) {
    return operators.find(s) != operators.end();
}

// In function body
opp_count = std::count_if(rdo_num_stack.begin(), rdo_num_stack.end(), is_operator);

Likewise, counting the parentheses in your stack could also be implemented using count_if(), rather than incrementing bcount in your iteration:

bcount = std::count_if(rdo_num_stack.begin(), rdo_num_stack.end(), []
                             (const std::string& value)
                             {
                                 return (value == "(" || value == ")");
                             });

Don't write code that does nothing

You also have written (verbatim):

expr = ""; // line 417 in original

for (i = 0; i < rdo_num_stack.size();i++) {
    expr += rdo_num_stack[i];
}

expr = "";

This assigns the contents of rdo_num_stack to expr, and then immediately overwrites expr. Everything but the last assignment can be removed.

Deep nesting

Deeply nested code is hard to read, and possibly to reason about. Several times you have a construct with a for-loop and nested if` conditions:

for (/* loop conditions */) { // summarized from line 134 in original
    if (!condition1) { 
        if (condition2) {
            // Loop body
        } else {
            // report error and quit
        }
    }
}

You can reduce the level of nesting by inverting the conditional logic and breaking early:

for (/* loop conditions */) {
    if (condition1) {continue;}
    if (!condition2) { /* report error and quit*/}
    // Loop body
}

StackExchange Code Review Q#57107, answer score: 13