patterncppMinor
Stack based recursive arithmetic expression parser
Viewed 0 times
expressionstackparserrecursivebasedarithmetic
Problem
I recently started to learn C++ and came across this problem in a book "Object Oriented Programming in C++" by Robert Lafore.
The author has a program in the book which creates a Parser. I found a problem with the original program in the book which breaks at one of the input expressions I am providing. Therefore I tried to make changes and made it recursive. It works fine with the inputs I am providing.
I am more concerned about correctness and programming style of the program I have written. I believe I have made many mistakes. Any comments would be very helpful.
```
#include
using namespace std;
const char SIZE = 40;
class Stack
{
private:
char array[SIZE];
char top;
public:
Stack():top(-1)
{
}
void push(char v)
{
if (!is_full())
{
array[++top] = v;
}
}
char pop()
{
if (!is_empty())
{
return array[top--];
}
throw "Stack is empty. Pop failed";
}
bool is_full()
{
return top >= SIZE ? true : false;
}
bool is_empty()
{
return top == -1;
}
char get_top()
{
return top;
}
};
bool is_operator(char c)
{
return (c == '*' || c == '/' || c == '+' || c == '-');
}
bool is_dm_operator(char c)
{
return (c == '*' || c == '/');
}
bool is_value(char c)
{
return (c >= '0' && c (result) (expected_result) ("5 / 5 + 3 - 6 * 2"), // why static_cast here? what is const_cast
static_cast("3 * 7 - 1 + 5 / 3"),
static_cast("3 * 5 - 4"),
static_cast("3 + 5 - 4"),
static_cast("2 / 6 * 3 / 2"),
static_cast("3 + 6 * 9 / 3 - 7"),
static_cast("9 - 5 / 5 * 2 + 6"),
static_cast("7 + 3 4 / 2 - 5 6"),
static_cast("4 * 5 + 3 - 4 / 2"),
The author has a program in the book which creates a Parser. I found a problem with the original program in the book which breaks at one of the input expressions I am providing. Therefore I tried to make changes and made it recursive. It works fine with the inputs I am providing.
I am more concerned about correctness and programming style of the program I have written. I believe I have made many mistakes. Any comments would be very helpful.
```
#include
using namespace std;
const char SIZE = 40;
class Stack
{
private:
char array[SIZE];
char top;
public:
Stack():top(-1)
{
}
void push(char v)
{
if (!is_full())
{
array[++top] = v;
}
}
char pop()
{
if (!is_empty())
{
return array[top--];
}
throw "Stack is empty. Pop failed";
}
bool is_full()
{
return top >= SIZE ? true : false;
}
bool is_empty()
{
return top == -1;
}
char get_top()
{
return top;
}
};
bool is_operator(char c)
{
return (c == '*' || c == '/' || c == '+' || c == '-');
}
bool is_dm_operator(char c)
{
return (c == '*' || c == '/');
}
bool is_value(char c)
{
return (c >= '0' && c (result) (expected_result) ("5 / 5 + 3 - 6 * 2"), // why static_cast here? what is const_cast
static_cast("3 * 7 - 1 + 5 / 3"),
static_cast("3 * 5 - 4"),
static_cast("3 + 5 - 4"),
static_cast("2 / 6 * 3 / 2"),
static_cast("3 + 6 * 9 / 3 - 7"),
static_cast("9 - 5 / 5 * 2 + 6"),
static_cast("7 + 3 4 / 2 - 5 6"),
static_cast("4 * 5 + 3 - 4 / 2"),
Solution
- Don't use
using namespace std;
While that would work in your particular case, it's considered bad practice. Especially when you move out your code to separate header files.
See more details here please:
Why is “using namespace std;” considered bad practice?
- Don't reinvent the wheel
You shouldn't write your own stack class, in preference of just using what the C++ standard library already provides.
There's already
std::stack, that should work well for your needs.- Don't use raw arrays in C++
Your raw array
char array[SIZE];should be a
std::array arr;instead.
std::array would cover all your needs, and gives you a more safe implementation.Also the naming
array might clash with the using namespace std; as mentioned.- Don't use
throwwith types not inherited fromstd::exception
throw "Stack is empty. Pop failed";This is a bad practice example of using a
throw statement. While it can be caught in various ways likecatch(const char* e) {
// handle exception
std::cerr << e << std::endl;
exit(1);
}it shouldn't be done like this. You will loose any distinction about certain categories of exceptions that could occur (again: Don't reinvent the wheel. Get familiar with the C++ standard library instead).
The C++ exception hierarchy is based on
std::exception, and what you actually have is a runtime failure. So you should rather use the standard exception class that indicates that:throw std::runtime_error("Stack Class Error: Stack is empty. Pop failed";The above exception could be caught transparently and reported using the
what() function:catch(const std::exception& e) {
// handle exception
std::cerr << e.what() << std::endl;
exit(1);
}- Use the correct cast operations
static_cast("5 / 5 + 3 - 6 * 2"), // why static_cast here? what is const_castYour guts were right about that. At least that even doesn't need a
const_cast since the character literal "5 / 5 + 3 - 6 2" already decays to a const char[] / const char anyways.Code Snippets
char array[SIZE];std::array<char,SIZE> arr;throw "Stack is empty. Pop failed";catch(const char* e) {
// handle exception
std::cerr << e << std::endl;
exit(1);
}throw std::runtime_error("Stack Class Error: Stack is empty. Pop failed";Context
StackExchange Code Review Q#162628, answer score: 5
Revisions (0)
No revisions yet.