Here are some things that may help you improve your code.
Use include guards
There should be an include guard in each .h file. That is, start the file with:
#ifndef EXPRESSION_H
#define EXPRESSION_H
// file contents go here
#endif // EXPRESSION_H
The use of #pragma once is a common extension, but it's not in the standard and thus represents at least a potential portability problem. See SF.8
Consider using standard routines
C++ provides a great number of useful utilites such as std::remove that you could use instead of your own hand-written function to remove whitespace. They're almost always faster and less buggy and by using library software, you can develop your own code more quickly and accurately. So instead of this:
//Remove whitespace
std::string output;
for (auto &i : s)
{
if (i != ' ')
output += i;
}
return parseRec(output);
One could use this:
s.erase(std::remove(s.begin(), s.end(), ' '), s.end());
return parseRec(s);
Note that this uses s rather than output per the next suggestion.
Understand pass-by-value
One thing that is quite different about C++ as compared with Java is how parameter passing is done. In general, there is pass-by-value and pass-by-reference. The pass-by-value is what we get when we write things like this:
static Expression parse(std::string s);
The gives parse its own copy of s. If we use pass-by-reference, it would generally look something like this:
static Expression parse(const std::string& s);
Now we are giving parse a reference (indicated by &) rather than copying it. In this example, we use the very common idiom of passing a const reference. This indicates that the called function parse may examine but not alter the value. Unlike Java, there is no automatic garbage collector in C++, so it's up to you to manage memory. Generally, passing by reference is faster and uses less memory, but in cases where we would be making a copy anyway, passing by value makes sense.
Prefer private to public where practical
The Expression class has its only data members as public members. Rather than do it that way, it would be better to keep them private. See C.9 for details.
Eliminate redundant functions
When a non-static member function is called in a C++ class, *this is implicitly passed. So the result is that this function is entirely useless:
float Expression::eval()
{
return Expression::evalRec(*this);
}
Instead, just rename evalRec as eval and write it as a member function:
float Expression::eval()
{
switch (symbol[0])
{
case '+':
return left->eval() + right->eval();
case '-':
return left->eval() - right->eval();
case '*':
return left->eval() * right->eval();
case '/':
return left->eval() / right->eval();
default:
return std::stoi(symbol);
}
}
Simplify your code by making full use of standard library structures
The parseRec routine uses a std::ordered_map and also a switch to find the operator with the lowest precedence. I suggest that when you see that you have repeated the operators in multiple places, this is a hint that perhaps you could use a different approach to avoid this duplication. In this case, my inclination would be to use std::min_element. This requires beginning and ending iterators and a comparison operation. First, let's write a comparison operation:
enum class op_precedence { LOW, MED, NON_OP };
static constexpr op_precedence precedence(char ch) {
op_precedence val{op_precedence::NON_OP};
switch (ch) {
case '+':
case '-':
val = op_precedence::LOW;
break;
case '*':
case '/':
val = op_precedence::MED;
break;
default:
break;
}
return val;
}
Now we can use std::min_element:
std::string::iterator lowest{std::min_element(s.begin(), s.end(),
[](const char &a, const char &b){
return precedence(a) < precedence(b);
})
};
This uses a lambda to actually effect the comparison. So now the whole function looks like this:
Expression Expression::parseRec(std::string s) {
std::string::iterator lowest{std::min_element(s.begin(), s.end(),
[](const char &a, const char &b){
return precedence(a) < precedence(b);
})
if (precedence(*lowest) == op_precedence::NON_OP) {
return Expression(s, nullptr, nullptr);
} else {
return Expression(std::string(1, *lowest),
new Expression(parseRec(s.substr(0, lowest-s.begin()))),
new Expression(parseRec(s.substr(lowest-s.begin() + 1)))
);
}
}
Note, too, that this uses the modern nullptr rather than the old C-style NULL.
Dive deeper into C++
I know you're just beginning with C++ (and it looks like a promising beginning!) but to whet your appetite for more, we can actually go a little further with the concept mentioned above. Specifically, we have a similar duplication in eval, so we could further consolidate information. Here's one way to do it:
enum class op_precedence { LOW, MED, NON_OP };
struct op {
op_precedence precedence;
float (*operation)(float, float);
};
static const std::unordered_map<char, op> operation = {
{ '+', { op_precedence::LOW, [](float a, float b){return a+b;}} },
{ '-', { op_precedence::LOW, [](float a, float b){return a-b;}} },
{ '*', { op_precedence::MED, [](float a, float b){return a*b;}} },
{ '/', { op_precedence::MED, [](float a, float b){return a/b;}} },
};
static op_precedence precedence(char ch) {
auto myop{operation.find(ch)};
return myop == operation.end() ? op_precedence::NON_OP : myop->second.precedence;
}
Now the rest of the program gets shorter and more concise:
float Expression::eval()
{
auto myop{operation.find(symbol[0])};
if (myop == operation.end()) {
return std::stoi(symbol);
}
return myop->second.operation(left->eval(), right->eval());
}
Expression Expression::parse(std::string s)
{
s.erase(std::remove(s.begin(), s.end(), ' '), s.end());
return parseRec(s);
}
Expression Expression::parseRec(std::string s)
{
std::string::iterator lowest{std::min_element(s.begin(), s.end(), [](const char &a, const char &b){
return precedence(a) < precedence(b); })};
if (precedence(*lowest) == op_precedence::NON_OP)
{
return Expression(s, nullptr, nullptr);
}
else
{
return Expression(std::string(1, *lowest),
new Expression(parseRec(s.substr(0, lowest-s.begin()))),
new Expression(parseRec(s.substr(lowest-s.begin() + 1)))
);
}
}
Don't use std::endl if you don't really need it
The difference betweeen std::endl and '\n' is that '\n' just emits a newline character, while std::endl actually flushes the stream. This can be time-consuming in a program with a lot of I/O and is rarely actually needed. It's best to only use std::endl when you have some good reason to flush the stream and it's not very often needed for simple programs such as this one. Avoiding the habit of using std::endl when '\n' will do will pay dividends in the future as you write more complex programs with more I/O and where performance needs to be maximized.
About return 0;
All compliant compilers will generate the equivalent of return 0 at the end of main, so I generally omit that. Others prefer to write it explicitly for stylistic reasons. Whichever you choose, it's useful to know.
Use the appropriate smart pointer type
The code, as written, abuses std::shared_ptr because no two objects should ever actually share the same Expression. For that reason, what you really should use instead is std::unique_ptr, but that makes things a bit more complex. In particular, we can no longer freely make copies (or they wouldn't be unique!) so we must use C++ move semantics. It's all perhaps a bit much for a beginner, but here's how one would rewrite the relevant portions of the class declaration.
std::unique_ptr<Expression> left;
std::unique_ptr<Expression> right;
Expression(std::string symbol, std::unique_ptr<Expression> &&left, std::unique_ptr<Expression> &&right);
Now the constructor looks like this:
Expression::Expression(std::string symbol, std::unique_ptr<Expression> &&left, std::unique_ptr<Expression> &&right)
: symbol(symbol), left(std::move(left)), right(std::move(right)) {}
And instead of using new (which is rather rare in modern C++), we use std::make_unique instead:
return Expression(std::string(1, *lowest),
std::make_unique<Expression>(parseRec(s.substr(0, lowest-s.begin()))),
std::make_unique<Expression>(parseRec(s.substr(lowest-s.begin() + 1)))
);
