1

I found some code to make a C implementation of stacks, and decided to use it. However, there were several typedefs, and I am having difficulty printing the values in a stackT (really a char array). Below is the code. What am I doing wrong?

#include <stdio.h>
#include <stdlib.h>

typedef char stackElementT;

typedef struct {
  stackElementT *contents;
  int maxSize;
  int top;
} stackT;

void StackInit(stackT *stackP, int maxSize) {
    stackElementT *newContents;
    newContents = (stackElementT *)malloc(sizeof(stackElementT)*maxSize);
    if (newContents == NULL) {
        fprintf(stderr, "Not enough memory.\n");
        exit(1);
    }

    stackP->contents = newContents;
    stackP->maxSize = maxSize;
    stackP->top = -1; //empty...
}

void StackDestroy(stackT *stackP) {
    free(stackP->contents);
    stackP->contents = NULL;
    stackP->maxSize = 0;
    stackP->top = -1; //empty
}

int StackIsEmpty(stackT *stackP) {
    return stackP->top < 0;
}

int StackIsFull(stackT *stackP) {
    return stackP->top >= stackP->maxSize-1;
}

void StackPush(stackT *stackP, stackElementT element) {
    if(StackIsFull(stackP)) {
        fprintf(stderr, "Can't push element: stack is full.\n");
        exit(1);
    }
    stackP->contents[++stackP->top] = element;
}

stackElementT StackPop(stackT *stackP) {
    if(StackIsEmpty(stackP)) {
        fprintf(stderr, "Can't pop element: stack is empty.\n");
        exit(1);
    }
    return stackP->contents[stackP->top--];
}

void StackDisplay(stackT *stackP) {
    if(StackIsEmpty(stackP)) {
        fprintf(stderr, "Can't display: stack is empty.\n");
        exit(1);
    }
    int i;
    printf("[ ");
    for (i = 0; i < stackP->top; i++) {
        printf("%c, ", stackP[i]); //the problem occurs HERE
    }
    printf("%c ]", stackP[stackP->top]);
}

int postfix(char* expr, int length) {
    int i;
    stackT stack;
    StackInit(&stack, 1000);
    int temp;
    for (i = 0; i < length; i++) {
        if ((expr[i] >= 48) && (expr[i] <= 57)) {
            printf("Is a number! Pushed %d\n", expr[i]);
            StackPush(&stack, expr[i]);
        }
        else {
            switch (expr[i]) {
                case 43: {
                    temp = StackPop(&stack);
                    StackPush(&stack, StackPop(&stack)+temp);
                }
                    break;
                case 45: {
                    temp = StackPop(&stack);
                    StackPush(&stack, StackPop(&stack)-temp);
                }
                    break;
                case 47: {
                    temp = StackPop(&stack);
                    StackPush(&stack, StackPop(&stack)/temp);
                }
                    break;
                case 42: {
                    temp = StackPop(&stack);
                    StackPush(&stack, StackPop(&stack)*temp);
                }
                    break;
                default:
                    break;
            }
        }
    }
    return StackPop(&stack);
}

int main() {
    int i;
    char* expr = "1 2 3 + * 3 2 1 - + *";
    for(i = 0; expr[i] != '\0'; i++) ;
    printf("%d\n", postfix(expr, i));
}
0

1 Answer 1

13

The compiler (GCC 4.2.1 on MacOS X 10.6.7) tells me:

$ cc -O -std=c99 -Wall -Wextra     st.c   -o st
st.c: In function ‘StackDisplay’:
st.c:72: warning: format ‘%c’ expects type ‘int’, but argument 2 has type ‘stackT’
st.c:74: warning: format ‘%c’ expects type ‘int’, but argument 2 has type ‘stackT’
$

In my version of the code, these two lines are the printf() statements in StackDisplay(), right where you state you have problems.

void StackDisplay(stackT *stackP)
{
    if(StackIsEmpty(stackP)) {
        fprintf(stderr, "Can't display: stack is empty.\n");
        exit(1);
    }
    int i;
    printf("[ ");
    for (i = 0; i < stackP->top; i++) {
        printf("%c, ", stackP[i]); //the problem occurs HERE
    }
    printf("%c ]", stackP[stackP->top]);
}

You probably want stackP->contents[i]. With that fix, the program 'runs' but produces:

Can't pop element: stack is empty.

That is your problem to resolve, now.

(Oh, I also fixed the stray semi-colon after the for loop in main() as diagnosed in the comments.)

The loop should be written as strlen(expr) (and then you need to #include <string.h>). Indeed, the body of the main program simplifies to:

char* expr = "1 2 3 + * 3 2 1 - + *";
printf("%d\n", postfix(expr, strlen(expr)));

You should normally keep top indexed to the next location to use, so the initial value would normally be 0 rather than -1.

Don't learn the ASCII codes for the digits - forget you ever did.

    if ((expr[i] >= 48) && (expr[i] <= 57)) {

You should write:

    if ((expr[i] >= '0') && (expr[i] <= '9')) {

or, better (but you have to #include <ctype.h> too):

    if (isdigit(expr[i])) {

Similar comments apply to the switch:

        switch (expr[i]) {
            case 43: {
                temp = StackPop(&stack);
                StackPush(&stack, StackPop(&stack)+temp);
            }
                break;

I'm not sure of the logic behind the indentation, but that 43 should be written as '+', 45 as '-', 47 as '/', and 42 as'*'.


This generates:

Is a number! Pushed 49
Is a number! Pushed 50
Is a number! Pushed 51
Is a number! Pushed 51
Is a number! Pushed 50
Is a number! Pushed 49
68

If you fix the number pushing code as shown:

printf("Is a number! Pushed %d\n", expr[i] - '0');
StackPush(&stack, expr[i] - '0');

Then you get:

Is a number! Pushed 1
Is a number! Pushed 2
Is a number! Pushed 3
Is a number! Pushed 3
Is a number! Pushed 2
Is a number! Pushed 1
20

And with a little more instrumentation, along the lines of:

temp = StackPop(&stack);
printf("Sub: result %d\n", temp);
StackPush(&stack, temp);

after each operation, the result is:

Is a number! Pushed 1
Is a number! Pushed 2
Is a number! Pushed 3
Add: result 5
Mul: result 5
Is a number! Pushed 3
Is a number! Pushed 2
Is a number! Pushed 1
Sub: result 1
Add: result 4
Mul: result 20
20

You were close.

Sign up to request clarification or add additional context in comments.

3 Comments

Fantastic answer, I wish I had an extra +1 to give you. Props to you for taking the time to get this fellow on the right track.
Wow. This was above and beyond the requirements, and I thank you very much for it. As @adam says, I wish I could give you more than a +1.
Truly comprehensive answer! Kudos! Especially like that your version pushes/pops values instead of characters... Would like to point out that a trivial 'fix' would escalate the limited range of -128->127 to higher values if typedef char stackElementT; was int instead of signed chars. As it is, " 9 9 * 2 * " is going to give a wrong result.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.