I have a problem with 2 functions that I have created for a program the uses the stack. However, I can’t be sure if there is an error in two functions at once, because they are interconnected. Thus, when adding a data to the stack and in its subsequent output, I get not quite the correct output.
input the 1 th element :1
input the 2 th element :2
input the 3 th element :3
So if the input goes like this, when i choose the 3rd option to output the data in stack i got such output
the elements in the stack are: 0
0
0
3
0
Here is the code
#include<stdio.h>
#include<string.h>
#include<stdlib.h>
#define MAXSIZE 10
int i=1,choose;
/* i represents the number of inputted elements; choose represents the identifiers of the options in the menu.*/
int *sptr,*full,*empty;
int stack[MAXSIZE];
void push(void);
void pop(void);
void printInfo(void);
int main(){
sptr=stack; // sptr points to stack[0].
empty=stack; //empty points to stack[0]
full=stack MAXSIZE-1; // full points to stack[9]
do{
printf("\n\t===============STACK ==============\n");
printf("\n\t 1.Push stack");
printf("\n\t 2.Pop stack");
printf("\n\t 3.Print elements of the stack");
printf("\n\t 4.Exit\n");
printf("\n\t Please choose[1-4] :");
scanf("%d",&choose);
switch(choose){
case 1:
push();
break;
case 2:
pop();
break;
case 3:
printInfo();
break;
case 4:
exit(0);
default:
printf("\n\n\t==================Input error=================");
break;
}
}while(1);
return 0;
}
void push(void){
sptr=stack 1; // sptr point to the next position of the array
if(sptr==full){
printf("\n\n ........The stack is full.......");
sptr--;
}else{
printf("input the %d th element : ",i );
scanf("%d",sptr);
}
}
void pop(void){
if(sptr!=empty){
sptr--;
i--;
}else{
printf("\n\n\t\t ........the stack is empty.......");
i=1;
}
}
void printInfo(void){
int * temp;
temp=sptr;
printf("\n\n\t the elements in the stack are: ");
do{
if(temp!=empty){
for(i;i>=0;i--)
printf("%d\n",stack[i]);
temp--;
}else{
break;
}
}while(1);
printf("\n\n\t================END===============\n");
}
I would be grateful for any advice or criticism. Thank you.
CodePudding user response:
With sptr=stack 1;
you always write to the same position in stack.
You need to increment it instead of always setting it to stack 1.
sptr=sptr 1;
or
sptr ;
This matches what you did inside pop()
: sptr--;
.
That gets you an output of
===============STACK ==============
1.Push stack
2.Pop stack
3.Print elements of the stack
4.Exit
Please choose[1-4] :1
input the 1 th element : 1
===============STACK ==============
1.Push stack
2.Pop stack
3.Print elements of the stack
4.Exit
Please choose[1-4] :1
input the 2 th element : 2
===============STACK ==============
1.Push stack
2.Pop stack
3.Print elements of the stack
4.Exit
Please choose[1-4] :1
input the 3 th element : 3
===============STACK ==============
1.Push stack
2.Pop stack
3.Print elements of the stack
4.Exit
Please choose[1-4] :3
the elements in the stack are: 0
3
2
1
0
I.e. the core of the push/pop functionality is fixed.
I think you might also want to avoid the 0 in output which was not entered for pushing.
For that I recommend inside printInfo()
:
while(temp!=empty)
{
printf("%d\n",*temp);
temp--;
}
which gets you a final output of:
the elements in the stack are: 3
2
1
which can use some work on whitespace, but only outputs the actually pushed values.
So yes, one function was functionally wrong, the other at least misleadingly unhelpful.
CodePudding user response:
There is a lot going on here, and you seem to have overcomplicated things and got confused. Let's take it a piece at a time:
#define MAXSIZE 10
int i=1,choose;
/* i represents the number of inputted elements; choose represents the identifiers of the options in the menu.*/
int *sptr,*full,*empty;
int stack[MAXSIZE];
Other than for acedemic reasons, there isn't much point in maintaining pointers (sptr, full, empty) and count (i) for indexing into the storage (stack). You are in effect storing the same information in different places (sptr
is analogous to i
, full
is analogous to &stack[MAXSIZE]
, and empty
is stack
), and that sort of habit will lead to divergence and bugs that are hard to track down.
Whilst pointer arithmetic is possible in C, its error prone and a common source of bugs. i
gives us all the information we need to be able to know how much stack
is in use.
Also, array indexing starts at 0
in C, (its an offset from base), and even with your existing logic, initialising it to 1
when there are no elements is patently going to lead to problems.
So that leaves us:
#define MAXSIZE 10
int stack[MAXSIZE];
int i=0;
NB: choose
has no business being global when its only used in the scope of main, so I'm leaving that out for now.
The following is now redundant and should be deleted:
sptr=stack; // sptr points to stack[0].
empty=stack; //empty points to stack[0]
full=stack MAXSIZE-1; // full points to stack[9]
The rest of main is ok, except for adding the definition of choose
int main() {
int choose;
//...
We can now address each of the functions in turn.
void push(void){
if(i < MAXSIZE) {
printf("input the %d th element : ",i);
scanf("%d",&stack[i ]);
}else{
printf("\n\n ........The stack is full.......");
}
}
Notes:
- Personal preference, but I prefer to show the 'happy path' first, and it simplifies the comparison slightly.
- Post-increment of
i
means that wheni == 0
,0
is used as the index intostack
, and the&
converts the expression a pointer to the value at the offset ofi
(which is whatscanf
needs as to where to store the value).
void pop(void) {
if(i > 0) {
i--;
} else {
printf("\n\n\t\t ........the stack is empty.......");
}
}
Notes:
- Again, happy path first, its a lot simpler now ;-)
void printInfo(void){
printf("\n\n\t the elements in the stack are: ");
for (int pos=0; pos < i ; pos ) {
printf("%d\n", stack[pos]);
}
printf("\n\n\t================END===============\n");
}
Notes:
- Remembering that we don't want to be modifying
i
when printing, we introduce a new variable,pos
, to step through the elements instack
, up to the current count.
Complete code:
#include<stdio.h>
#include<string.h>
#include<stdlib.h>
#define MAXSIZE 10
int i=1;
int stack[MAXSIZE];
void push(void);
void pop(void);
void printInfo(void);
int main() {
int choose;
do{
printf("\n\t===============STACK ==============\n");
printf("\n\t 1.Push stack");
printf("\n\t 2.Pop stack");
printf("\n\t 3.Print elements of the stack");
printf("\n\t 4.Exit\n");
printf("\n\t Please choose[1-4] :");
scanf("%d",&choose);
switch(choose){
case 1:
push();
break;
case 2:
pop();
break;
case 3:
printInfo();
break;
case 4:
exit(0);
default:
printf("\n\n\t==================Input error=================");
break;
}
}while(1);
return 0;
}
void push(void) {
if (i < MAXSIZE) {
printf("input the %d th element : ",i);
scanf("%d",&stack[i ]);
} else {
printf("\n\n ........The stack is full.......");
}
}
void pop(void) {
if (i > 0) {
i--;
} else {
printf("\n\n\t\t ........the stack is empty.......");
}
}
void printInfo(void) {
printf("\n\n\t the elements in the stack are: ");
for (int pos=0; pos < i ; pos ) {
printf("%d\n", stack[pos]);
}
printf("\n\n\t================END===============\n");
}
If you want too, it should be fairly simple to modify push
, pop
, and printInfo
to take a pointer to stack
and the count (i
) so that you can move the globals into the scope of main and pass them to the helpers. This reduces globals and means that its easier to reason as to what push, pop and printInfo act upon (only the inputs, and not globals).
And that will teach you a bit more about pointers, without leading to bad habits.
CodePudding user response:
About for(i;i>=0;i--)
in your void printInfo(void)
, first you can't use variable "i", because after you use printInfo to print information of stack then global variable "i" will be 0, but there are elements in it.
Second, your i doesn't mean the number of inputted elements, i-1 means the number of inputted elements.
And Yunnosch's answer is also right, there are many problems in your code. You need to check it carefully.