HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Stack based recursive arithmetic expression parser

Submitted by: @import:stackexchange-codereview··
0
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"),

Solution


  1. 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?

  1. 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.

  1. 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.

  1. Don't use throw with types not inherited from std::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 like

catch(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);
}


  1. Use the correct cast operations



static_cast("5 / 5 + 3 - 6 * 2"), // why static_cast here? what is const_cast


Your 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.