Mathematical expressions evaluator with callbacks, the logic

I hesitated quite a bit of time before posting this question since the code to review is more or less a monster (with regards to its size). It is a basic mathematical expressions evaluator, but with one interesting additional feature: you can register callbacks that can be used in the expression to evaluate. First of all, here is a little example of how it works (taken straight from the documentation):

evaluator eval;

std::cout << eval("2 + 5 * 3") << '\n'; // 17
std::cout << eval("(4 + 2) * (2 ** 3)") << '\n'; // 48
std::cout << eval("5!") << '\n'; // 120

// Connect the absolute value function
eval.connect("abs", [](int n) {
    return std::abs(n);
});

std::cout << eval("2 * abs(2 - 8) + 4") << '\n'; // 16

If it makes you feel better, you might want to consider this question to be a late answer to the April 2015 Community Challenge since this is what motivated me to update my old evaluator in the first place :)

The evaluator

So, first things first: the code revolves around an evaluator object. This object has the ability to connect and disconnect callbacks and to evaluate mathematical expressions with either the method evaluate or operator() so that it can also be used as a function object. Here is the evaluator class. Note that for the purpose of the question, the body of the functions is embedded in the classes but in the real code, the implementation is in a separate .inl/.cpp file:

```
template
class evaluator
{
public:

/**
* @brief Evaluate a mathematical expression.
*/
auto evaluate(const std::string& expression) const
-> Number
{
auto tokens = tokenize(expression);
return eval_postfix(to_postfix(tokens));
}

/**
* @brief Calls evaluate.
*/
auto operator()(const std::string& expression) const
-> Number
{
return evaluate(expression);
}

/**
* @brief Regist

Suspected Bugs:

A typo I think

case '|': // | or ||
            if (it[1] == '*')

Stylistic differences:

A couple of things I would do differently.

But I believe you and I differ on these minor stylistic things:

User defined Types

I prefer to use an initial caps when defining types. It helps distinguish types from objects.

New Style auto return

I still prefer the old style of function declaration over the new style.

// old style
std::vector> tokenize(const std::string& expr)

// new style
auto tokenize(const std::string& expr) -> std::vector>

I use the new style only when the return type depends on the inputs.

Lexer

I would have used a lexer tool. That's how I roll.

Hard To spot break

case '-': // - (unary or binary)
            if (res.empty())
            {
                // STUFF
                break
            }
            else
            {
                // STUFF
                break;
            }

Personally I would move the breaks outside the if statement. So it is clear that the case statement does have a break.

case '-': // - (unary or binary)
            if (res.empty())
            {
                // STUFF
            }
            else
            {
                // STUFF
            }
            break;

Same thing in:

case '!': // ! (prefix or postfix) and !=

Changes that may help:

Limit of 16 parameters

Number params[16];
                    //  STUFF
                    operands.push(func(params));
                    break;

Why not use a std::vector?

You can still pass it as an array pointer by taking the address of the first argument.

std::vector x;
                    //  STUFF
                    operands.push(func(¶ms[0]));
                    break;

New Stuff:

Not seen this before

callbacks.emplace(
            std::piecewise_construct,     // Will have to go read about this.
            std::forward_as_tuple(name),
            std::forward_as_tuple(function)
        );

Would using move semantics help for future


output.push(operations.top());

In the future it may be more than just numbers that you use in the expression. You want to prepare for that by using move semantics when transfering values in a couple of places.

output.push(std::move(operations.top()));

StackExchange Code Review Q#91586, answer score: 5