Thread: Stack operations from switch statement elements

I have a program that compiles with no errors or warnings, but the logic is flawed. It is a postfix calculator...I have a default in the switch statement that is supposed to match non-operator input and post it.

The switch is called as a result of a function to define isoperator conditions.

Unfortunately, the input is read, and each char input, space, and \n is printf'd back out as the invalid input default and a random math total is displayed.

Am I misinterpreting the logic contained within my switch statement?

Code:

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <ctype.h>
#define STACKSIZE 256
#define TRUE 1
#define FALSE 0

/* stack type definition */

typedef struct {
   int top;
   float items[STACKSIZE];
} stack_type;

/* function prototypes */
int is_empty(stack_type*);
void push(stack_type*, float);
float pop(stack_type*);


int main(void)
{
   char buffer[256];
   char *input[STACKSIZE];
   stack_type stk;
   int i=0;
   stk.top = -1;            /* Initialize the stack to empty */
   int sum;
   printf("This program will evaluate postfix expressions using a\n");
   printf("good implementation of stacks.\n");
   printf("You may enter up to a 50-char expression.\n");
   printf("Please enter a postfix/reverse-polish expression for me.\n");
   fgets(*input, sizeof(*input), stdin);
   while(1) {
   for (i=0; i<strlen(*input); i++) {
   int operator = is_operator(input[i]);
   if(operator)
   {
      switch(operator) {

         case '+':
            sum = pop(&stk) + pop(&stk);
         break;
         case '-':
            sum = pop(&stk) - pop(&stk);
         break;
         case '*':
            sum = pop(&stk) * pop(&stk);
         break;
         case '/':
            sum = pop(&stk) / pop(&stk);
         break;
         case '%':
            sum = pop(&stk) % pop(&stk);
         break;
         default:
            printf("Epic Fail...invalid input.\n");
      }
         push(&stk, sum);
   }
   else if (isdigit(*input[i]))
      {
         push(&stk, (*input[i] - 48));
      }
   else if (strcmp(*input, "quit\n") == 0)
      {
         printf("Time to quit...goodbye!\n");
         exit(0);
      }
  }

printf("The total is %f\n", stk);
printf("Please enter another expression: \n");
fgets(*input, sizeof(*input), stdin);
}
return 0;
}


/* IS Empty */
int is_empty(stack_type* stk)
{
   if ( (*stk).top == -1 )
      return TRUE;
   else
      return FALSE;
}


/* Push */
void push(stack_type* stk, float element)
{
   if ( stk == NULL )
   {  printf("Warning! Stack pointer is set to NULL.\n");
      exit(1); /* exit the program */
   }

   /* check stack is not full */
   if ( (*stk).top == (STACKSIZE - 1) )
   {  printf("Stack Overflow!\n");
      exit(1);
   }

   /* increment stack top */
   (*stk).top++;

   /* add the new element to the top */
   (*stk).items[(*stk).top] = element;

   return;
}
/* Pop */
float pop(stack_type* stk)
{  float value=0;
   /* Check stack exists */
   if ( stk == NULL )
   {  printf("Warning!  Stack pointer is set to NULL.  Is it really there?\n");
      exit(1);
   }
   /* check that the stack is not empty */
   if (is_empty(stk))
   {  printf("Stack underflow!\n");
      exit(1);
   }
   /* get top item */
   value = (*stk).items[(*stk).top];
   /* decrement top */
   (*stk).top--;
   /* return popped item to calling f(x); */
   return value;
}
int is_operator(int x)
{
  if(x == '+' || x == '-' || x =='/' || x == '*' || x == '%')
    return;
}

1. Your is_operator doesn't return what you want it to (as in it currently doesn't return anything).
2. Your input handling routine assumes single digit numbers and no spaces. If that's not true, then that for-loop you have on i is in a lot of trouble.

It should return x, mlsrar. I don't believe I wrote it incorrectly when I had given you some other code. I kind of liked the function pointers but I see you didn't opt to use that method. Is this calculator supposed to use polish notation?

By the way, one huge limitation to your current code is that it can only do single digit math. Is that what you are wanting?

Originally Posted by tabstop

1. Your is_operator doesn't return what you want it to (as in it currently doesn't return anything).
2. Your input handling routine assumes single digit numbers and no spaces. If that's not true, then that for-loop you have on i is in a lot of trouble.

Code:

int is_operator(int x)
{
  if(x == '+' || x == '-' || x =='/' || x == '*' || x == '%')
    return x;
}

I am assuming single-digit input, but was not assuming spaces. Tabstop, thanks for your help.

Originally Posted by master5001

It should return x, mlsrar. I don't believe I wrote it incorrectly when I had given you some other code. I kind of liked the function pointers but I see you didn't opt to use that method. Is this calculator supposed to use polish notation?

No, I think I missed it in the cut/paste...it is in there. I'm just doing a single-digit implementation, as I'm learning it, and didn't want to tackle the complications. It IS supposed to use polish notation. The good news...now I'm seg-faulting!

I'll post updated code shortly.

Example:

Code:

int is_operator(int x)
{
  if(x == '+' || x == '-' || x =='/' || x == '*' || x == '%')
    return x;
  return 0;
}

The other line you didn't copy and paste is equally important. By the way, there was no way in hell that your compiler compiled the code as it was before. Either your compiler is about as useful as a one legged horse or you need to stop ignoring warnings it issues.

Originally Posted by master5001

Example:

Code:

int is_operator(int x)
{
  if(x == '+' || x == '-' || x =='/' || x == '*' || x == '%')
    return x;
  return 0;
}

The other line you didn't copy and paste is equally important. By the way, there was no way in hell that your compiler compiled the code as it was before. Either your compiler is about as useful as a one legged horse or you need to stop ignoring warnings it issues.

Again, my terminal and opera aren't cooperating. Return 0 is there.

I have no need to declare input as a pointer. It compiles and executes, but always returns 2 as a sum:

Code:

int main(void)
{
   char buffer[256];
   char input[STACKSIZE];
   stack_type stk;
   int i=0;
   stk.top = -1;            /* Initialize the stack to empty */
   int sum;
   printf("This program will evaluate postfix expressions using a\n");
   printf("good implementation of stacks.\n");
   printf("You may enter up to a 50-char expression.\n");
   printf("Please enter a postfix/reverse-polish expression for me.\n");
   fgets(input, sizeof(input), stdin);
   while(1) {
   for (i=0; i<strlen(input); i++) {
   int operator = is_operator(input[i]);
   if(operator)
   {
      switch(operator) {

         case '+':
            sum = pop(&stk) + pop(&stk);
         break;
         case '-':
            sum = pop(&stk) - pop(&stk);
         break;
         case '*':
            sum = pop(&stk) * pop(&stk);
         break;
         case '/':
            sum = pop(&stk) / pop(&stk);
         break;
         /*case '%':
            sum = pop(&stk) % pop(&stk);
         break;*/
         default:
            printf("Epic Fail...invalid input.\n");
      }
         push(&stk, sum);
   }
   else if (isdigit(input[i]))
      {
         push(&stk, (input[i] - 48));
      }
   else if (strcmp(input, "quit\n") == 0)
      {
         printf("Time to quit...goodbye!\n");
         exit(0);
      }
  }

printf("The total is %f\n", stk);
printf("Please enter another expression: \n");
fgets(input, sizeof(input), stdin);
}
return 0;
}

What should it be returning?

Lets say I type +22
Your code should return 4, right?

And why the infinite loop?

Originally Posted by master5001

What should it be returning?

Lets say I type +22
Your code should return 4, right?

And why the infinite loop?

Code:

  else if (strcmp(input, "quit\n") == 0)
      {
         printf("Time to quit...goodbye!\n");
         exit(0);
      }
  }

If quit is input, i break out of the loop.

It's a basic postfix calculator...add items to the stack, follow the order of operations. 2 2 + would push 4 back onto the stack.

Ah ok, right. But that is not my understanding of how it is gathering stuff... Why not do this...

Example:

Code:

char *p;

// later on...

for (p = input; *p; p++) {
   int operator = is_operator(*p);

   if(operator)
   {
      switch(operator) {
         case '+':
            sum = pop(&stk) + pop(&stk);
         break;
         case '-':
            sum = pop(&stk) - pop(&stk);
         break;
         case '*':
            sum = pop(&stk) * pop(&stk);
         break;
         case '/':
            sum = pop(&stk) / pop(&stk);
         break;
         /*case '%':
            sum = pop(&stk) % pop(&stk);
         break;*/
         default:
            printf("Epic Fail...invalid input.\n");
      }
         push(&stk, sum);
   }
   else if (isdigit(*p))
      {
         push(&stk, strtol(p, &p, 10));
      }
   else if (strcmp(p, "quit\n") == 0)
      {
         printf("Time to quit...goodbye!\n");
         exit(0);
      }
   else
      continue;
  }

I don't know if your braces are lined up correctly. I don't really care, just extract what is useful.

Someplace you have to have real memory allocated. I would assume something like

char input[STACKSIZE]

is adequate. Then *p is used to trundle thru the string a character at a time. Or you could use input[i] and increment i instead of incrementing a pointer that way.

Originally Posted by nonoob

Someplace you have to have real memory allocated. I would assume something like

char input[STACKSIZE]

is adequate. Then *p is used to trundle thru the string a character at a time. Or you could use input[i] and increment i instead of incrementing a pointer that way.

As in the way I"m currently implementing it?

Code:

   char input[STACKSIZE];
   stack_type stk;
   int i=0;
   stk.top = -1;            /* Initialize the stack to empty */
   int sum;
   printf("This program will evaluate postfix expressions using a\n");
   printf("good implementation of stacks.\n");
   printf("You may enter up to a 50-char expression.\n");
   printf("Please enter a postfix/reverse-polish expression for me.\n");
   fgets(input, sizeof(input), stdin);
   while(1) {
   for (i=0; i<strlen(input); i++) {
   int operator = is_operator(input[i]);
   if(operator)
   {

Looks good to me.