HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Simple Stack Implementation in C

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
implementationsimplestack

Problem

This is my implementation of the stack data type in C language. It is static meaning it does not grow in size. Any suggestions or feedback would be much appreciated.

```
#include
#define MAXSTACKLENGTH 6
#define true 1
#define false 0

struct stk{
char content[MAXSTACKLENGTH];
int top;
};

typedef struct stk stack;

int size(stack *s)
{
return s->top;
}

int isEmpty(stack *s)
{
if (size(s) -1)
{
return false;
}
}

int isFull(stack *s) {
if (size(s) == (MAXSTACKLENGTH-1))
{
return true;
}
else {
return false;
}
}

char peek(stack *s)
{
if (isEmpty(s) == true)
{
printf("Can't peek: Stack is empty!\n");
return '\0';
}
else {
return s->content[s->top];
}
}

void push(stack *s, char element)
{
if (isFull(s) == true)
{
printf("Cant push: Stack is full!\n");
}
else {
s->top++;
s->content[s->top] = element;
}
}

void pop(stack *s)
{
if (isEmpty(s) == true) {
printf("Cant pop: Stack is empty!\n");
}
else {
s->top--;
}
}

void input_push(stack *s)
{
char c;
printf("Enter a character> ");
scanf(" %c", &c);

push(s, c);
}

void print_stack(stack *s)
{
if (isEmpty(s)) {
printf("Stack is empty!\n");
}
else {
int i = s->top;

for (i; i > -1; i--)
{
printf("%c\n", s->content[i]);
}

printf("\n");
}
}

void pause()
{
char c;
printf("Enter any character to continue.\n");
scanf(" %c", &c);
}

int main()
{
printf("SimpleStack! v1\n");

stack mystack;
mystack.top = -1; //Empty

int run = true;
int choice = 0;

while (run)
{
if (isFull(&mystack))
{
printf("Warning -> Stack is full\n");
}
if (isEmpty(&mystack))
{
printf("Warning -> Stack is empty\n");
}
printf("1 -> Push\n");
printf("2

Solution

It looks good. Here are a few things you can add to it or change:

-
You don't have to define true/false yourself. It's unfortunate that bool is not a built-in type in C, but the Standard C Library at least provides an #include file that gives you those definitions in a portable way. So you can #include and it will define the type bool and true/false constants.

-
You can combine the structure declaration + typedef into the same declaration:

typedef struct stk 
{
    char content[MAXSTACKLENGTH];
    int top;
} stack;


Most C programs you'll find use this more compact notation of typedef struct.

-
You can simplify statements like this and others:

int isEmpty(stack *s)
{
    if (size(s)  -1)
    {
        return false;
    }
}


By simply returning the condition:

int isEmpty(stack *s)
{
    return size(s) <= -1;
}


The result of a comparison is always a boolean, so it can be returned directly if the function also returns true/false. This is another pattern that you will see used a lot. It makes code more concise.

-
Comparing against true/false in a conditional is not very usual:

if (isFull(s) == true)


You gave good descriptive names to your functions, so you can just test them like this:

if (isFull(s)) { ...


And to test if not full, the more usual way is to apply the ! (logical not) operator:

if (!isFull(s)) { ... // Reads "if not isFull"


-
size() currently return -1 for an empty stack. That's unusual. It doesn't make sense for a size to be negative. I'd expect an empty stack to have size 0. You should change that.

-
I suggest having a init_stack() function that sets the expected initial invariants when a new stack is declared. Better than having to remember to manually set top to -1, for example.

-
Since you already have a custom pause() function, you don't have to use system("pause"). The latter is non-portable and not recommended for use in real-life programs due to security risk it might incur. Best if you avoid it.

-
A minor nitpick, you're mixing different styles of curly brace positioning. Some are on the same line as the parent conditional, some are on their own line. This creates a break in your style, which is a little annoying. It would be nice if you kept them consistent, independent of which style you decide for.

Code Snippets

typedef struct stk 
{
    char content[MAXSTACKLENGTH];
    int top;
} stack;
int isEmpty(stack *s)
{
    if (size(s) <= -1)
    {
        return true;
    }
    else if (size(s) > -1)
    {
        return false;
    }
}
int isEmpty(stack *s)
{
    return size(s) <= -1;
}
if (isFull(s) == true)
if (isFull(s)) { ...

Context

StackExchange Code Review Q#114812, answer score: 8

Revisions (0)

No revisions yet.