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 == "=")
{
...