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

Algebraic calculator

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
calculatoralgebraicstackoverflow

Problem

I started with my own implementation of an algebraic calculator but ran into issues very early into the process. Then, I implemented the algorithm here. Please provide some inputs on areas where I can improve.

```
#include
#include
#include
#include

class infixProcessor{

public:
infixProcessor(std::string input){
inputString = input;
}
private:
std::string inputString;
std::string postfixExpr;
std::stack stk;
std::stack postStk;

bool isNumber (char inputValue){
if (inputValue >=48 && inputValue <= 57) return true;
else return false;
}

bool isOpenParen(char inputValue){
if (inputValue == '(') return true;
else return false;
}

bool isCloseParen(char inputValue){
if (inputValue == ')') return true;
else return false;
}

bool isOperator(char inputValue){
if (inputValue == '+' || inputValue == '-') return true;
else return false;
}

bool isSpace(char inputValue){
if (inputValue == ' ') return true;
else return false;
}
public:
void process(){

inputString.erase(std::remove(inputString.begin(),inputString.end(),' '),inputString.end());
std::string::iterator strIt;
for (strIt = inputString.begin() ; strIt != inputString.end() ; strIt++){
char currentValue = *strIt;
if (isNumber(currentValue)){
postfixExpr += currentValue;

//Peek ahead, if next character is part of previous number
while ( isNumber(*(strIt+1)) ){
postfixExpr += *(strIt+1);
strIt++;
}

postfixExpr += ' ';

}
else if (isOperator(currentValue)){
while ( !stk.empty() && isOperator(stk.top()) ){
postfixExpr += stk.top();
postfixExpr += ' ';
stk.pop();
}
s

Solution

When comparing number in a text string you can use '0' and '9' etc...

bool isNumber (char inputValue){
    if (inputValue >=48 && inputValue <= 57) return true;
    else return false;
}


Is easier to read as:

bool isNumber (char inputValue){
    if (inputValue >= '0' && inputValue <= '9') return true;
    else return false;
}


Not only easier to read but the types are correct.

When you want to return true/false from an if expression better to just use the value of the expression.

bool isNumber (char inputValue){
    if (inputValue >= '0' && inputValue <= '9') return true;
    else return false;
}


Is easier to read as:

bool isNumber (char inputValue){
    return (inputValue >= '0' && inputValue <= '9');
}


Note if you are testing for a number it is more efficient to use the standard functions.

bool isNumber (char inputValue){
    return std::isdigit(inputValue);  // :-)
         //
         // This basically is inlined as an array lookup.
         //    return isdigitarray[inputValue];
}


Range based for.

for (strIt = inputString.begin() ; strIt != inputString.end() ; strIt++){


In C++11 we introduced a range based for. It works with any type that supports begin() and end()

for (auto const& value: inputString){

Code Snippets

bool isNumber (char inputValue){
    if (inputValue >=48 && inputValue <= 57) return true;
    else return false;
}
bool isNumber (char inputValue){
    if (inputValue >= '0' && inputValue <= '9') return true;
    else return false;
}
bool isNumber (char inputValue){
    if (inputValue >= '0' && inputValue <= '9') return true;
    else return false;
}
bool isNumber (char inputValue){
    return (inputValue >= '0' && inputValue <= '9');
}
bool isNumber (char inputValue){
    return std::isdigit(inputValue);  // :-)
         //
         // This basically is inlined as an array lookup.
         //    return isdigitarray[inputValue];
}

Context

StackExchange Code Review Q#100433, answer score: 8

Revisions (0)

No revisions yet.