Search code examples
c

Why does my getop function in C return '+' correctly but not '=' when there are leading spaces?


I was trying to do one coding questions solve, that says:

Exercise 4-3. Given the basic framework, it's straightforward to extend the calculator. Add the modulus (%) and unary minus operators. Add an "erase" command which erases the top entry on the stack. Add commands for handling variables. (Twenty-six single-letter variable names is easy.)

this exercise had main(), pop(), push(), clear() functions already there, I had to make changes to them according to the exercise said, add getop() function in it too as it was not there so I did add it as my understanding of getop(), here is the code (not completed)

/* Exercise 4-3. Given the basic framework, it's straightforward to 
   extend the calculator. Add the modulus (%) and unary minus 
   operators. Add an "erase" command which erases the top entry on
   the stack. Add commands for handling variables. (Twenty-six 
   single-letter variable names is easy.) 
*/

#include <stdio.h>

#define MAXOP 20 /* max size of operand, operator */
#define NUMBER '0' /* signal that number found */
#define TOOBIG '9' /* signal that string is too big */
#define MAXVAL 100 /* maximum depth of val stack */

int sp = 0; /* stack pointer */
double val[MAXVAL]; /* value stack */

void clear() /* clear stack */
{
  sp = 0;
}

double push(double f) /* push f onto value stack */
{
  if (sp < MAXVAL)
    return (val[sp++] = f);
  else {
    printf("error: stack full\n");
    clear();
    return (0);
  }
}

double pop() /* pop top value from stack */
{
  if (sp > 0)
    return (val[--sp]);
  else {
    printf("error: stack empty\n");
    clear();
    return (0);
  }
}

int getop(char s[], int lim) {
    int c, blank = 0, i = 0;

    while ((c = getchar()) == '\n' || c == '\t' || c == ' ') 
        blank++;

    if ((c < '0' || c > '9') && c != '.' && blank == 0)
        return c;
    else
    if ((c >= '0' && c <= '9') || c == '.') {
        do {
            s[i++] = c;
        } while ((((c = getchar()) >= '0' && c <= '9') || c == '.') && i < (lim - 1));
    }

    s[i] = 0;

    if (i > 0 && i < (lim - 1))
        return NUMBER;
    else if (i == (lim - 1))
        return TOOBIG;
}

int main() /* reverse Polish desk calculator */
{
  int type;
  char s[MAXOP];
  double op2, atof(), pop(), push();

  while ((type = getop(s, MAXOP)) != EOF) {
    printf("type: %c\n", type); /* prints type before switch case */
    switch (type) {

    case NUMBER:
        push(atof(s));
        break;
    case '+':
        push(pop() + pop());
        break;
    case '*':
        push(pop() * pop());
        break;
    case '-':
        op2 = pop();
        push(pop() - op2);
        break;
    case '/':
        op2 = pop();
        if (op2 != 0.0)
          push(pop() / op2);
        else
          printf("zero divisor popped\n");
        break;
    case '=':
        printf("\t%f\n", push(pop()));
        break;
    case 'c':
        clear();
        break;
    case TOOBIG:
        printf("%.20s ... is too long\n", s);
        break;
    default:
        printf("unknown command '%c'\n", type);
        break;
    }
  }
}

so when the input is 3 4 + = the terminal view is

c_projects/ nvim main1.c
c_projects/ gcc main1.c -o main
c_projects/ ./main
3 4 + =
type: 0
type: 0
type: +
type:
unknown command ''
^C
c_projects/

so as you see I am printing int type as %c before switch case to see what getop() returns to main() and its fine for '3', '4', '+' but why it sends ' ' that is before '=' but doesn't send spaces before previous inputs.

Its fine when input is 3 4 += with no space between '+' and '=' and here is the terminal view

c_projects/ ./main
3 4 +=
type: 0
type: 0
type: +
type: =
        7.000000
^C
c_projects/

I have checked it running on online compiler to check if it is not some undefined behavior but no it is same on online compiler too.


Solution

  • There are multiple problems:

    • you only return the operator if you did not skip any blanks. Yet you only consume the character that follows a number, not whatever follows on operator. Hence you return + but not =. Remove the && blanks == 0 part of the test.
    • there is no test for EOF in the do / while loop. This is a classic mistake in this kind of loop. These loops are very error prone, you should not use them at all. Savvy programmers use do / while loops in macros to encapsulate multiple statements into a single one, but as a beginner you should not write macros either (unless you are requested to).
    • when you parse a number, you consume the character that follows the number, which happens to be a space in the examples posted... but you should instead push back this character into the input stream with ungetc(), making spaces between numbers and operators optional.
    • the second operator = read after a space causes all following tests to be false and make the function exit without a return statement, causing undefined behavior. The value returned is not printable as shown in the session posted.

    Lessons learned:

    • do not use do/while loops
    • push back characters that you do not use
    • compile with warnings enabled to let the compiler help you find silly mistakes: use -Wall -Wextra with gcc and clang.

    Here is a modified version:

    int getop(char s[], size_t lim) {
        int c;
    
        while ((c = getchar()) == '\n') || c == '\t' || c == ' ') 
            continue;
    
        if ((c >= '0' && c <= '9') || c == '.') {
            size_t i = 0;
            int dot = 0;
            while ((c >= '0' && c <= '9') || c == '.') {
                if (c == '.' && dot++)  // only accept a single '.'
                    break;
                if (i + 1 >= lim) {
                    ungetc(c, stdin);
                    return TOOBIG;
                }
                s[i++] = (char)c;
                c = getchar();
            }
            ungetc(c, stdin);
            s[i] = '\0';
            return NUMBER;
        }
        return c;  // operator or EOF
    }