patterncModerate
Stack implementation using an array
Viewed 0 times
arrayimplementationusingstack
Problem
I am trying a stack implementation using an array. I want to know if this approach is OK or if there is a logical problem. This program is working fine.
```
#include
#define MAXSIZE 5
struct stack / Structure definition for stack /
{
int stk[MAXSIZE];
int top;
} s;
void push ();
void pop();
void display ();
void search();
int main(){
int element, choice;
s.top = -1;
clrscr();
while (1)
{
printf ("------------------------------------------\n");
printf ("1. PUSH\n");
printf ("2. POP\n");
printf ("3. DISPLAY\n");
printf ("4. SEARCH\n");
printf ("5. EXIT\n");
printf ("------------------------------------------\n");
printf ("Enter your choice\n");
scanf ("%d", &choice);
switch (choice)
{
case 1: push();
break;
case 2: pop();
break;
case 3: display();
break;
case 4: search();
break;
case 5: return;
}
}
}
/Function to add an element to the stack/
void push() {
int num;
if (s.top == (MAXSIZE - 1))
{
printf ("Error: Overflow\n");
}
else
{
printf ("Enter the element to be pushed\n");
scanf ("%d", &num);
s.top = s.top + 1;
s.stk[s.top] = num;
}
}
/Function to delete an element from the stack/
void pop ()
{
int num;
if (s.top == - 1)
{
printf ("Error: Stack Empty\n");
}
else
{
num = s.stk[s.top];
printf ("poped element is = %d\n", num);
s.top = s.top - 1;
}
}
/Function to display the status of the stack/
void display()
{
int i;
if (s.top == -1)
{
printf ("Error: Stack Empty\n");
}
else
{
printf ("\nItems in Stack\n");
for (i = s.top; i >= 0; i--)
{
printf ("%d\n", s.stk[i]);
}
}
printf ("\n");
}
void sea
```
#include
#define MAXSIZE 5
struct stack / Structure definition for stack /
{
int stk[MAXSIZE];
int top;
} s;
void push ();
void pop();
void display ();
void search();
int main(){
int element, choice;
s.top = -1;
clrscr();
while (1)
{
printf ("------------------------------------------\n");
printf ("1. PUSH\n");
printf ("2. POP\n");
printf ("3. DISPLAY\n");
printf ("4. SEARCH\n");
printf ("5. EXIT\n");
printf ("------------------------------------------\n");
printf ("Enter your choice\n");
scanf ("%d", &choice);
switch (choice)
{
case 1: push();
break;
case 2: pop();
break;
case 3: display();
break;
case 4: search();
break;
case 5: return;
}
}
}
/Function to add an element to the stack/
void push() {
int num;
if (s.top == (MAXSIZE - 1))
{
printf ("Error: Overflow\n");
}
else
{
printf ("Enter the element to be pushed\n");
scanf ("%d", &num);
s.top = s.top + 1;
s.stk[s.top] = num;
}
}
/Function to delete an element from the stack/
void pop ()
{
int num;
if (s.top == - 1)
{
printf ("Error: Stack Empty\n");
}
else
{
num = s.stk[s.top];
printf ("poped element is = %d\n", num);
s.top = s.top - 1;
}
}
/Function to display the status of the stack/
void display()
{
int i;
if (s.top == -1)
{
printf ("Error: Stack Empty\n");
}
else
{
printf ("\nItems in Stack\n");
for (i = s.top; i >= 0; i--)
{
printf ("%d\n", s.stk[i]);
}
}
printf ("\n");
}
void sea
Solution
sscanf is not safe
It's not part of your stack but part of your driver code, but
Better, in this case, would be to use something like
separate stack operations from input/output
Having a function like
operations should operate on passed parameters
In this code, there can only be one stack because it's globally defined and used by all stack operation functions. A better scheme would be to pass the address of the stack structure to each function to make it more flexible and more general.
Additionally, a function like
use C idioms to make the code shorter and easier to read
Writing code such as
is not incorrect, but is not idiomatic C. The C way of writing that would be
Writing code in this way will make it easier for others to understand you, in the same way that speaking, say, idiomatic Italian will make it easier for native Italian speakers to understand you.
avoid non-portable code
The
I hope these items inspire you to write more code, and to strive to continuously improve the quality of the code that you write.
It's not part of your stack but part of your driver code, but
scanf() is not a good function to use. There are many reasons for this, but the basic problem in this case is that if the user enters a letter rather than a digit for input in push() for example, the program will loop forever.Better, in this case, would be to use something like
getchar().separate stack operations from input/output
Having a function like
push() only do one thing (pushing an item to the stack) is generally better design because it makes it easier to re-use your code. In this code, push() cannot be easily re-used because it is tied directly to user I/O. Better would be to cleanly separate all stack operations from the I/O to better foster re-use.operations should operate on passed parameters
In this code, there can only be one stack because it's globally defined and used by all stack operation functions. A better scheme would be to pass the address of the stack structure to each function to make it more flexible and more general.
Additionally, a function like
push() should also require an argument that speccifies what is to be pushed onto the stack, and a function like pop() should return either a copy of or a pointer to the data item that was popped from the stack.use C idioms to make the code shorter and easier to read
Writing code such as
s.top = s.top + 1;
s.stk[s.top] = num;is not incorrect, but is not idiomatic C. The C way of writing that would be
s.stk[++s.top] = num;Writing code in this way will make it easier for others to understand you, in the same way that speaking, say, idiomatic Italian will make it easier for native Italian speakers to understand you.
avoid non-portable code
The
clrscr() call is not in ` and is not portable. I had to comment out that line to get the code to compile and run. Ideally, you'd avoid all non-portable code. If you can't avoid that, however, you should state what dependencies the code has and also include the header files that clearly indicate those dependencies.
think about the user of your code
It's good that you avoided using "magic numbers" and instead you wisely chose to define MAXSIZE to indicate the size of your stack. However, it would be useful to have a comment indicating what it's for, and also to maybe rename it to something a little more descriptive, such as MAXSTACKSIZE.
avoid single-letter variable names
While i or j might be perfectly fine variable names for looping indexes, naming your stack s` is not very user-friendly. We are long beyond the days that each line of code had to fit on an 80-character punch card!I hope these items inspire you to write more code, and to strive to continuously improve the quality of the code that you write.
Code Snippets
s.top = s.top + 1;
s.stk[s.top] = num;s.stk[++s.top] = num;Context
StackExchange Code Review Q#46748, answer score: 14
Revisions (0)
No revisions yet.