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)); }
Improve this question
0

1 Answer 1

Reset to default
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.

Improve this answer
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.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.