Hi zakels
The problem in you original code is not that you're ignoring the spaces in your switch statement. Its because you main function exits after it processes the first char it gets.
Adding code to report all gets, all scope entrances and exits, and when spaces are found, the output I get for "30 + 3.2 * 4.5" is:
[begin main]
30 + 3.2 * 4.5
main got 3
[begin putTogether]
putTogether got 0
[begin putTogether]
putTogether got
putTogether found a space : fall through to default
[begin doNumber]
number 30
[end doNumber]
[end putTogether]
[end putTogether]
[end main] |
Note that the space case in main is not triggered.
Various comments on the original code:
1. return; is optional as the last statement in a function with a void return type. Most people would omit it.
2. multiple returns from a function should be generally avoided. It made it harder than necessary for me to add debug logging. Well, it was just a search and replace in the end, but I ended up with a log statement for each case statement whereas I should have just needed one at the end. Here it's better to use break; rather than return 0; for the case statements. e.g.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37
|
int putTogether(string numberStack, char ch)
{
#ifdef LOGGING
cout << "[begin putTogether]" << endl;
#endif
numberStack += ch;
ch = cin.get();
switch (ch)
{
case ')':
doNumber(numberStack);
doOperator(ch);
break;
case '(':
doNumber(numberStack);
doOperator(ch);
break;
// <snip>
case '0':
putTogether(numberStack, ch);
break;
case '1':
putTogether(numberStack, ch);
break;
// <snip>
case '.':
putTogether(numberStack, ch);
break;
case ' ':
default:
doNumber(numberStack);
}
#ifdef LOGGING
cout << "[end putTogether]" << endl;
#endif
return 0;
}
|
3. if multiple cases use use the same code, it is ok to merge them.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22
|
switch (ch)
{
case ')':
case '(':
case '+':
// <snip>
case '^':
doNumber(numberStack);
doOperator(ch);
break;
case '0':
case '1':
// <snip>
case '9':
case '.':
doNumber(numberStack);
break;
case ' ':
cout << "putTogether found a space : fall through to default" << endl;
default:
doNumber(numberStack);
}
|
But this suggests the same move to me as Azagaros suggests: to uses isdigit(). I suggest you define your own isoperator() function to make things consistent and tidy.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17
|
if(isoperator(ch))
{
doNumber(numberStack);
doOperator(ch);
}
else if(isdigit(ch) || (ch == '.'))
{
doNumber(numberStack);
}
else if(isspace(ch))
{
doNumber(numberStack); // not sure about this?
}
else
{
// handle error here!
}
|
Finally, you should note how your main switch statement and the one in putTogether are kind of similar. Ideally should should have just the one.
And you will need to track more state than the bNumberDone variable Azagaros' code uses, if you want to handle operator precedence and brackets.
Most exercise books get you to solve the postfix operator case first, and then the infix case. As is the case for your example, 30 + 3.2 * 4.5, the + operator cannot be applied until after the * operator. So you will need a way to store values and operators until you need to apply them.