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

Dynamic stack implementation

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

Problem

I wrote this code for a dynamic stack and would like to know what would be better if done differently.

It can increase memory by a percentage and/or fixed value when needed. It can also be set to not increase automatically at all, so slots are only allocated by the user.

The stack is just a structure holding any data type, so it can change what kind of data is stored without deallocating memory.

Header definitions

#include 
#include 
#include 

#define MINIMUM_ELEMENT_SIZE (1)
#define MULTIPLIER (1.00)
#define EXPANSION_AMOUNT (8)

typedef struct {
    void *content;
    size_t element_size;
    size_t count;
    size_t slots;
    float multiplier;
    size_t expansion_amount;
} Stack;


Some inline functions to do very simple things

static inline void *memory_from_position(Stack *stack, size_t position) __attribute__((always_inline));
static inline size_t required_memory_for_slots(Stack *stack, size_t n_slots) __attribute__((always_inline));
static inline size_t how_many_slots_can_fit(size_t memory_size, size_t slot_size) __attribute__((always_inline));
static inline size_t available_slots(Stack *stack) __attribute__((always_inline));

//Return memory address based on position supplied, starts at 0, no error
//checking, caller must be sure the position is in range
static inline void *memory_from_position(Stack *stack, size_t position)
{
    return stack->content + stack->element_size * position;
}

//Return how much memory n_slots would occupy
static inline size_t required_memory_for_slots(Stack *stack, size_t n_slots)
{
    return stack->element_size * n_slots;
}

//Return how many slots can fit into memory
static inline size_t how_many_slots_can_fit(size_t memory_size, size_t slot_size)
{
    return memory_size / slot_size;
}

//Return how many slots there are left
static inline size_t available_slots(Stack *stack)
{
    return stack->slots - stack->count;
}


Main functions

```
//Allocate and initialize stack
Stack *new_stack(size_t element_s

Solution

Why add the parentheses in your macro constants? This provides no benefit and may cause typos to compile (e.g. (fabs MULTIPLIER - 5) / EXPANSION_AMOUNT; when you meant to write fabs(MULTIPLIER - 5) / EXPANSION_AMOUNT;).

Remove __attribute__((always_inline)). Compilers these days are very good at optimizing, and overriding their decisions will almost never lead to better code. With these functions, the compiler will almost always inline them anyway, but there may be rare occasions when it determines that it is faster to produce code with function calls.

Your new_stack function is odd to me; it doesn't allocate content, and in fact allocates an object on the heap whose size is known at runtime when it may be more appropriate to allocate it on the stack! I'd pass it more parameters than just element_size, and have a separate init_stack function that initializes a stack that is passed in by reference in case the stack structure is allocated on the stack instead of the heap.

Actually, I would rename all of the functions in this library; since C has no namespaces, function names in a library ought to have library-specific names: stack_new, stack_push, stack_pop, etc.

You may want slots to be a derived value instead of a primary value, and instead store the size of the buffer in bytes. As it is,

Stack* s = new_stack(10);
add_slots(s, 10);
printf("Slots: %d\n", s->slots);
repurpose_stack(s, 15);
repurpose_stack(s, 10);
printf("Slots: %d\n" s->slots);
repurpose_stack(s, 91);
repurpose_stack(s, 10);
printf("Slots: %d\n" s->slots);


will print out "10\n9\n0\n", not "10\n10\n10\n".

I recommend adding an assertion to remove_slots:

assert(amount slots);


It is a programming error to remove more slots than actually exist in the data structure. This will be compiled out when optimizing, but may help find bugs while debugging.

Similarly, consider adding assertions in push and pop that check bounds.

Code Snippets

Stack* s = new_stack(10);
add_slots(s, 10);
printf("Slots: %d\n", s->slots);
repurpose_stack(s, 15);
repurpose_stack(s, 10);
printf("Slots: %d\n" s->slots);
repurpose_stack(s, 91);
repurpose_stack(s, 10);
printf("Slots: %d\n" s->slots);
assert(amount <= stack->slots);

Context

StackExchange Code Review Q#38085, answer score: 4

Revisions (0)

No revisions yet.