Stack of integers in C

Question

I'm learning C, and currently I follows C99 standard. Here is my implementation of integer-stack, which is just a stack you can do: push, pop, and print the content of the stack.

Here are some problems I currently have:

1. Is that #define STACK_MAX 1000 a good idea? Should I consider make it configurable in runtime?
2. It's impossible to make struct int_stack * to int_stack *, like C++, right?

Other than these two problem, I need feedbacks about anything you think important, but I missed out, thank you.

---

int_stack.h:

#ifndef INT_STACK_H
#define INT_STACK_H
#include <stdio.h>
#include <stdbool.h>
#define STACK_MAX 1000
typedef struct int_stack {
 int top;
 int data[STACK_MAX];
 bool (*push)(struct int_stack *, int);
 bool (*pop)(struct int_stack *, int *);
 int (*size)(struct int_stack *);
 void (*print_stack)(struct int_stack *);
} int_stack;
bool push(int_stack *self, int item) {
 if (self->top == STACK_MAX)
 return false;
 self->data[self->top++] = item;
 return true;
}
bool pop(int_stack *self, int *out) {
 if (size(self) == 0)
 return false;
 *out = self->data[--self->top];
 return true;
}
int size(int_stack *self) {
 return self->top;
}
void print_stack(int_stack *self) {
 printf("| top |\n");
 for (int i = self->top-1; i >= 0; i--)
 printf("| %3d |\n", self->data[i]);
}
void array_zeros_init(int A[], int size) {
 for (int i = 0; i < size; i++)
 A[i] = 0;
}
void stack_init(struct int_stack *stack) {
 stack->top = 0;
 array_zeros_init(stack->data, STACK_MAX);
 stack->push = &push;
 stack->pop = &pop;
 stack->size = &size;
 stack->print_stack = &print_stack;
}
#endif // !INT_STACK_H

• SomeProgrammer: Maybe make your function pointers static ? I don’t know much about c though, more of a c++ guy...

Answer 1

We wouldn't normally put the function definitions in the header like that. Just write the declarations that are needed by the calling code, and put the definitions into their own source file.

It seems odd to have these function pointers:

bool (*push)(struct int_stack *, int);
bool (*pop)(struct int_stack *, int *);
int (*size)(struct int_stack *);
void (*print_stack)(struct int_stack *);

Nothing needs them, so just omit them.

pop() uses size() before the latter is defined. We could solve this by re-ordering, but separating declarations from definitions will naturally fix this.

size() and print() don't need to modify the stack, so they should accept pointer to const object.

The function names are all vulnerable to collisions. I'd suggest adding a common prefix to distinguish and group them.

Writing zeros to the unused elements is a bit pointless, as those elements are not reachable through the public interface.

Here's my re-write:

Header:

#ifndef INT_STACK_H
#define INT_STACK_H
#include <stdbool.h>
typedef struct int_stack int_stack;
int_stack *stack_create(void);
void stack_free(int_stack *self);
bool stack_push(int_stack *self, int item);
bool stack_pop(int_stack *self, int *out);
int stack_size(const int_stack *self);
void stack_print(const int_stack *self);
#endif // !INT_STACK_H

Implementation

#include <stdio.h>
#include <stdlib.h>
#define STACK_MAX 1000
struct int_stack {
 int top;
 int data[STACK_MAX];
};
int_stack *stack_create(void) {
 int_stack *stack = malloc(sizeof *stack);
 if (stack) {
 stack->top = 0;
 }
 return stack;
}
void stack_free(int_stack *self) {
 free(self);
}
bool stack_push(int_stack *self, int item) {
 if (self->top == STACK_MAX) {
 return false;
 }
 self->data[self->top++] = item;
 return true;
}
bool stack_pop(int_stack *self, int *out) {
 if (stack_size(self) == 0) {
 return false;
 }
 *out = self->data[--self->top];
 return true;
}
int stack_size(const int_stack *self) {
 return self->top;
}
void stack_print(const int_stack *self) {
 printf("| top |\n");
 for (int i = self->top-1; i >= 0; i--) {
 printf("| %3d |\n", self->data[i]);
 }
}