Recursive implementation of stack without arrays POP not working

Question

I'm trying to implement a LIFO stack recursively without using arrays. The program takes in strings and ints as input, and has a few commands -- namely push <int> pop empty top and quit.

Everything but pop works fine for me, and pop only works partially. It's fine if you're popping just one int, but beyond that it returns me a stack is empty even though it is not. I understand why this is happening, but I'm not sure how to fix it.

int stack(int top, int last) {  
  int m = read_symbol();
  if (m != READ_FAIL) {
    if (m == PUSH_SYMBOL) {
      int n = read_int();
      top = stack(n, top);
    } else if (m == POP_SYMBOL) {
      if (top == INT_MIN) {
        printf("pop error - stack is empty\n");
        top = stack(INT_MIN, INT_MIN);
      } else {
        top = stack(last, INT_MIN);
      }
    } else if (m == TOP_SYMBOL) {
      if (top == INT_MIN) {
        printf("top error - stack is empty\n");
      } else {
        printf("top - %d\n", top);
      }
      top = stack(top, last);
    } else if (m == EMPTY_SYMBOL) {
      if (top == INT_MIN) {
        printf("stack is empty\n");
      } else {
        printf("stack is not empty\n");
      }
      top = stack(top, last);
    } else if (m == QUIT_SYMBOL) {
      if (top != INT_MIN) {
        printf("quit error - stack is not empty\n");
        top = stack(top, last);
      } else {
        printf("goodbye\n");
      }
    }
  }
  return top;
}

The top variable is recursively returned so everything works fine. But when I do something like

push 1
push 2
push 3
top
pop
top
pop
top

the output returned is

top - 3
top - 2
top error - stack is empty (SHOULD BE 1)

ive tried various different approaches but i havent been able to solve it. In fact I introduced the last parameter just to try and solve this, the rest of the implementation works fine even without last but this parameter for now seems to work but only for one pop command because the next recursion level sets last to INT_MIN which is then set to top if you pop again, hence the false stack is empty message

any pointers or help would be appreciated.

EDIT: INT_MIN refers to the C99 limits.h INT_MIN which is -(2^32 - 1)

why not? how would i listen for other commands like further `push` commands or the `quit` command? — bone bot, Jun 09 '19 at 18:50
sorry about that, `read_symbol` reads in a string and assigns a static int to it. Essentially, it is used to determine what command the input was — bone bot, Jun 09 '19 at 18:55
It does not however read the integer part of the string, which is why `push` contains an extra `read_int` which reads the integer value — bone bot, Jun 09 '19 at 18:56
@bonebot I'm not asking for a description (even though that's also a good thing). I'm asking for code. Edit your question and, as I said, provide a [mre] — klutt, Jun 09 '19 at 19:00
This seems like a great time to [learn how to debug your programs](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/). — Some programmer dude, Jun 09 '19 at 19:07
unfortunately this is an ungraded question in one of my university books and `read_symbol` is provided to us as a function defined in our header+binary C file, so I dont have access to it's code. I figure its probably using `scanf`. given just this code, may it be possible to give me some pointers and i can try to solve it myself? — bone bot, Jun 09 '19 at 19:21
@pmg if i return without a recursive call then the program quits — bone bot, Jun 09 '19 at 19:22

Work of Artiz · Answer 1 · 2019-06-09T19:35:34.817

0

So I think I've solved why it does this, first of all I slightly rewrote your function to use a switching statement for compactness (this is not the solution to your problem):

int stack(int top, int last) {
  switch(read_symbol()) {

    // push n
    case PUSH_SYMBOL:  
      return stack(read_int(), top);

    // pop
    case POP_SYMBOL:
      if (top == INT_MIN) {
        printf("pop error - stack is empty\n");
        return stack(INT_MIN, INT_MIN);
      }

      return stack(last, INT_MIN);

    // top  
    case TOP_SYMBOL:
      if (top == INT_MIN)
        printf("top error - stack is empty\n");
      else
        printf("top - %d\n", top);

      return stack(top, last);

    // empty
    case EMPTY_SYMBOL: 
      if (top == INT_MIN)
        printf("stack is empty\n");
      else
        printf("stack is not empty\n");

      return stack(top, last);

    // quit
    case QUIT_SYMBOL:
      if (top != INT_MIN) {
        printf("quit error - stack is not empty\n");
        return stack(top, last);
      }

      printf("goodbye\n");

    case READ_FAIL: // error handling
    default:
      return top;
  }
}

Then I followed the presumed call stack:

stack(INT_MIN, INT_MIN) receives PUSH 1 -> stack(1, INT_MIN)
stack(1, INT_MIN)       receives PUSH 2 -> stack(1, 2)
stack(1, 2)             receives PUSH 3 -> stack(3, 2)
stack(3, 2)             receives TOP    -> stack(3, 2)
stack(3, 2)             receives POP    -> stack(2, INT_MIN)
stack(2, INT_MIN)       receives TOP    -> stack(2, INT_MIN)
stack(2, INT_MIN)       receives POP    -> stack(INT_MIN, INT_MIN)
stack(INT_MIN, INT_MIN) receives TOP    => ERROR

The problem here is very simple, pop calls stack(x, INT_MIN), which in your code means, that after a pop, the stack is only of size 1 (or zero). The data from the previous call stack isn't used.

edited Jun 09 '19 at 19:35

answered Jun 09 '19 at 19:19

Work of Artiz

1,085
7
16

The problem stays the same. The last TOP should be returning `1` not `ERROR` – bone bot Jun 09 '19 at 19:27
Also I think the second stack trace is incorrect as after receiving `push 2` it should be `stack(2, 1)` -- `top` is the top element, and `last` is the one below top – bone bot Jun 09 '19 at 19:28
Oh this code doesn't solve the problem, it is correctly analysing where your program goes wrong – Work of Artiz Jun 09 '19 at 19:33
I understand where the problem is happening. I mentioned this in the OP. I'm not sure how I could fix it though. – bone bot Jun 09 '19 at 19:35
The problem simply lies in that every instruction recurses, you would want a PUSH call to call stack() again, the TOP and EMPTY calls to stay at the current level (no additional calls) and the POP to return directly. – Work of Artiz Jun 09 '19 at 19:37
if i ever return directly the program quits though, a recursive call is necessary to keep it running until `quit` is encountered – bone bot Jun 09 '19 at 19:41
Just use a loop, and return a DONE signal or a NOT_DONE signal from your function to know whether to quit – Work of Artiz Jun 09 '19 at 19:44
how would that work? i tried something similar before but then pop became useless as because of the loop a value was never returned to the next recursive levle – bone bot Jun 09 '19 at 19:54
Look, I won't go as far as writing a full solution, but what i was describing is that your function just taking a single int (top), which would be a while loop reading in commands, if its a push, you call the function recursively with new tos, if its TOP or EMPTY you just print whatever and continue in the loop, if its POP/QUIT you return (with not_done/done return val) – Work of Artiz Jun 09 '19 at 20:14
i tried various approaches but still cant get it to work – bone bot Jun 09 '19 at 23:40

ARD · Answer 2 · 2019-06-09T19:11:50.103

-4

inside else if (m == POP_SYMBOL) { } change

if (top == INT_MIN)

to

if (top < INT_MIN)

and let me know if it works for you as it can happen because of incorrect logic for the variable 'top' for e.g. we may think in our logic that the variable 'top' is breached, while actually it would be breached for the next iteration. It this doesn't work for you, then post value of INT_MIN (probably its a #define-ed value)

edited Jun 09 '19 at 19:11

answered Jun 09 '19 at 19:06

ARD

48
8

`INT_MIN` is defined when you `#include `. It is not necessarily the same for all implementations. – pmg Jun 09 '19 at 19:10
Thanks @Someprogrammerdude, for you quick comment, I updated my response to elaborate why I asked for the particular code change. :-) – ARD Jun 09 '19 at 19:13
@pmg, with the given code snippet, it is inconclusive that the programmer is including limits.h header file, and hence relying on values in that header file. Who knows that the coder might have simply defined her/his own value, hence I asked to post that value if the above code change doesn't work. Let's see how to him/her out of this – ARD Jun 09 '19 at 19:16
the INT_MIN is set to the default c99 limits.h INT_MIN value which i believe is -(2^32-1) -- `top` can never be less than that since that would be an underflow – bone bot Jun 09 '19 at 19:17
@bone_bot, the code snippet does not show limits.h being included. Please do not assume. Also, top is a user defined variable, there is no logic defined about how it is inited. So, please – ARD Jun 09 '19 at 19:19
It is difficult not to downvote this answer, because it suggest to insert a ridiculous statement into the program `if (top < INT_MIN)` where `top` is of type `int`, does not explain how this is supposed to improve things, and its author argues about the most implausible ways the change might be useful when it is pointed out that `if (top < INT_MIN)` is ridiculous. – Pascal Cuoq Jun 09 '19 at 19:24
2

The burden of proof is on you: it's your job to explain why the change would be beneficial, not on people who doubt your answer is the best suggestion to prove that the OP did not in fact `#define INT_MIN` to sometime an `int` can be lower than. Yes, it would also be nice if downvoters did not “simply down vote”, but I believe you have in fact gotten plenty of feedback on your answer. – Pascal Cuoq Jun 09 '19 at 19:29

Recursive implementation of stack without arrays POP not working

2 Answers2