Skip to main content
added 5 characters in body
Source Link
G. Sliepen
  • 69.3k
  • 3
  • 75
  • 180
ExpressionEvaluator square("x * x");
std::unordered_map<std::string, double> values;
auto& x = values["x"];
for (x = 0; x <= 10; ++x) {
    std::cout << x << "² = " << square(xvalues) << "\n";
}
ExpressionEvaluator square("x * x");
std::unordered_map<std::string, double> values;
auto& x = values["x"];
for (x = 0; x <= 10; ++x) {
    std::cout << x << "² = " << square(x) << "\n";
}
ExpressionEvaluator square("x * x");
std::unordered_map<std::string, double> values;
auto& x = values["x"];
for (x = 0; x <= 10; ++x) {
    std::cout << x << "² = " << square(values) << "\n";
}
Source Link
G. Sliepen
  • 69.3k
  • 3
  • 75
  • 180

For a beginner this already quite advanced and well-written code.

Improving custom exceptions

It's great that you have custom exceptions that derive from std::exception. However, std::exception has the drawback that you have to implement your own what(). Consider deriving from a higher level exception object instead, like std::runtime_error. Apart from providing more information about the nature of the error this way, you can then simplify the definition of your own custom exceptions like so:

struct parser_error: public std::runtime_error {
    parser_error(const std::string& text): std::runtime_error(text) {}
};

When catching errors, make use of the exception class hierarchy to avoid having to write multiple catch-statements:

try {
    std::cout << evaluator.evaluate(expression, std::move(variable_values)) << "\n";
} catch (const std::runtime_error& ex) {
    std::cerr << ex.what() << "\n";
}

When to use a class

If I look at the code in main(), I see:

ExpressionEvaluator evaluator;
std::cout << evaluator.evaluate(expression, variable_values);

What's the point of the evaluator object though? It doesn't seem like it provides any benefit. Why not have a free function, so you would just write:

std::cout << evaluate(expression, variable_values);

The _functions and _operators variables can be made static and put in a namespace.

A reason to do use a class would be if there is some state that you can reuse. For example, it would be really nice if you could create an evaluator for a given expression, and then call it multiple times with different values. For example, it would be really nice if you could write:

ExpressionEvaluator square("x * x");
std::unordered_map<std::string, double> values;
auto& x = values["x"];
for (x = 0; x <= 10; ++x) {
    std::cout << x << "² = " << square(x) << "\n";
}

The same goes for AstGenerator. Make generate_ast() a free function that returns an object of type Ast, the latter just being a container for an abstract syntax tree.

Use static variables

You are initializing a lot of maps at runtime in constructors. If you only ever create on object of a type that does that, it's not a problem, but consider that in larger projects, many of such objects might be created. You could go for a singleton pattern, but in C++ there is a much more straightforward solution, and that is to make these maps static, and initialize them at the same time you declare them. For example:

static std::unordered_map<Operator, OperatorDefinition> _unary_operator_definitions = {
    {Operator::add, {3,  Associativity::right}},
    {Operator::sub, {3,  Associativity::right}},
};
...
static std::unordered_map<Operator, std::function<double(const std::vector<double>&)>> _operators = {
    {Operator::add, [](auto& args) { ... }},
    {Operator::sub, [](auto& args) { ... }},
    ...
};

Unnecessary use of std::shared_ptr<>

You should use std::shared_ptr<> if you need shared ownership semantics. However, in your case that is not needed. The vector _tokens can be the sole owner of the tokens, and thus use std::unique_ptr<Token>. Then _operator_stack can just store raw pointers to the tokens owned by _tokens, since the latter's lifetime will be longer than that of the other containers. So:

std::vector<std::unique_ptr<Token>> _tokens;
st::stack<Token*> _operator_stack;

Consider using std::variant

Using inheritance and pointers is one way to store different types of tokens in a container, however since C++17 there is also std::variant which can be used as an alternative. For example, you could then just write:

using Token = std::variant<double, std::string, Operator>;
std::vector<Token> _tokens;

The main advantage would be that you don't need to use pointers anymore to store tokens. A disadvantage is that for a beginner, you might have to wrap your head around how to deal with variants. For example, see the "type matching visitor" example from the documentation of std::visit().