Calculating postfix notation using two forms of input

My postfix program essentially allows the user two input options to calculate a postfix equation:

• Confirmations after each input (after an input, the user is ask for another number, operator, or final result)

• No confirmation after each input (user is only asked for a number of inputs (values and operators), and the final result is shown after the loop)

My original attempt at this program used the first option because I had trouble getting my program to tell the difference between a value and an operator (both are treated as ASCII values, which is normal). However, I used a better method (the second option) after learning about atoi(). Alongside these two functions, another function does all of the calculations for both functions.

How could this program be improved for clarity and effectiveness? Should I even bother offering both options, even just for comparison's sake? I'm also not sure how much input validation I should provide, if it's really necessary.

NOTE: The IntStack class was written by an instructor of mine, and I'm just using it for simplicity and learning. As it is part of my own written code, I'm including it here so that my code's meaning is not lost. As it's not my own code, please do not review it. Only review my own code.

Postfix.cpp

```
#include
#include
#include "IntStack.h"
using std::cout;
using std::cin;

int noConfirmationExecution();
int confirmationExecution();
void calcOperation(IntStack&, char);

int main()
{
int executionType;
int calcTotal;

cout >> Execution Type:\n\n";
cout > executionType;

if (executionType == 1)
calcTotal = noConfirmationExecution();
else if (executionType == 2)
calcTotal = confirmationExecution();

cout > Items ('s' to stop):\n\n";

cin >> item;

while (item != 's')
{
int i = atoi(&item);

if (item != '+' && item != '-' && item != '*' && item != '/')
stack.push(i);
else if (ite

Sure if you have too:

using std::cout;
using std::cin;

But are you really saving that much.

Even forward declaration of functions, its nice to have the name of the parameters. It helps in understanding the context in which the function will be called.

void calcOperation(IntStack&, char);

I leave the parameter names off when they are not used (so usually this only happens in overridden virtual functions).

What happens if the user enters not one or two?

Personally I would make this a command line flag. Under normal conditions just run without validation. With the command line flag '-h' it means a human is working interactively and thus will confirm values.

Very platform specific:

system("PAUSE");

There is no real need for this. In a terminal the application quitting is not a problem. In an IDE you just have to set a configuration option to stop the window closing. So you really should not need to bother with this.

Personally I only return a value from main() if there is an option of failing.

return 0;

If the application always succeeds then no return is an indication that it will always work and thus no need to check for errors (note: in C++ main() is special and the compiler inserts return 0 if it does not exist).

Since you use the same stack in both situations. Why not declare it in main() and pass it in. Then if you need to modify it you only have one place to change (not two as in the current implementation).

IntStack stack(20);

Since item is a char. Any numbers can only be single digit. So using atoi() seems a waste. But it is also wrong. Because atoi() expects a '\0' terminated C-String (you have not provided this you need a second character set to '\0'

cin >> item;

        while (item != 's')
        {
            int i = atoi(&item);

To make this work you need:

cin >> item;
    while (item != 's)
    {
        int i = item - '0';

But to make it work with multi-characer numbers I think I would use std::string

std::string word;
    std::cin >> word;
    while (item != "s")
    {
        try
        {
            int i = boost::lexical_cast(word);
            stack.push(i);
        }
        catch(...)
        {
           // handle + - * / here as they are not numbers;
        }

If you can't use boost then the same affect can be achieved with std::stringstream and a little more work.

Your while loop looks like a for(;;) loop. So why not use it:

cin >> item;
        while(item != 's')
        {
            // STUFF
            cin >> item;
        }

        // I would write like this:
        for(cin >> item; item != 's'; cin >> item)
        {
        }

StackExchange Code Review Q#24324, answer score: 2