This bit of code here
if( calcStack.size() > 2 ) { cout << "Error: too many operands" << endl; return 1; }
Needs to move to main and slightly transform
else if(isOperator(input)) { performOperation(input, calcStack); } else if(input == "=") { if (calcStack.size() != 1) { cout << "Error: too many operands" << endl; return 1; } else { cout << "Result: " << calcStack.top(); // now decide whether or not you are preserving the result for // the next computation calcStack.pop(); // Assuming not keeping result } }
And that means you’ll need to rethink this loop while(input != "=")
You’re really close.
Two suggestions:
You can optimize the isOperator function.
bool isOperator(const string& input) { static const string operators ="-+*/"; if (input.length() == 1) // right size to be an operator. { return operators.find_first_of(input[0]) != string::npos; // look in the operator string for the first (and only) character in input } return false; }
And since you know there is only one character in the operator, you can use something more elegant than if/else if:
switch (input[0]) { case '-': result = firstOperand - secondOperand; break; case '+': result = firstOperand + secondOperand; break; case '*': result = firstOperand * secondOperand; break; case '/': if (secondOperand == 0) { // moved this test to here because it's the only place it matters. cout << "Error: Division by 0.\n"; return -1; } result = firstOperand / secondOperand; break; }
addendum to deal with comment
By this point your code should have changed quite a bit and you might want to start a new question with your current code. If not, this is what I’m talking about in the comments.
else if(isOperator(input)) { if (performOperation(input, calcStack) != 0) { // empty out the stack and delete all pending user input. calcStack.clear(); cin.ignore(numeric_limits<streamsize>::max(), '\n'); cout << "Bad input. Try again." << endl; break; // exit input loop. } } else if(input == "=") { ...