ADT stack with a dynamic array (revision 1)

Question 1

Here's the second draft of my ADT stack code which I posted herebefore after carrying out most of the improvements suggested there.

I decided to expose a function calledresize_stack to adjust the size of the stack as desired by the user instead of doing it automatically.

stacklib.h

#include <stdio.h>#include <stdlib.h>#include <errno.h>#ifndef STACKLIB_H#define STACKLIB_H/* generic type pushed item_to the stack */    /* in this case it's just an int */typedef int item_t;/* our stack data type */typedef struct {    size_t size;    size_t capacity;    item_t *elements;} stack;/* interface to our new stack data type */stack* create_stack(size_t capacity);int push_stack(stack *mystack, item_t data);item_t* pop_stack(stack *mystack);void delete_stack(stack *mystack);stack* resize_stack(stack *mystack, size_t capacity);#endif

stacklib.c

#include "stacklib.h"/* a helper function to handle exceptions */void handle_error(const char* message){    if(errno) {        perror(message);    } else {        printf("Error: %s\n", message);    }    exit(1);}stack* create_stack(size_t capacity){    stack *stack_ptr = malloc(sizeof(stack));    if(stack_ptr == NULL)        handle_error("Error while allocating memory.");    item_t *elements_ptr = malloc(capacity*sizeof(item_t));    if(elements_ptr == NULL) {        free(stack_ptr);        handle_error("Error while allocating memory.");    }    stack_ptr->size = 0;    stack_ptr->capacity = capacity;    stack_ptr->elements = elements_ptr;    return stack_ptr;}void delete_stack(stack *mystack){    free(mystack->elements);    free(mystack);}/* Returns 0 on success, 1 on failure */int push_stack(stack *mystack, item_t data){    /* stack is full! */    if(mystack->size == mystack->capacity)         return 1;    mystack->elements[mystack->size++] = data;    return 0;}/* Returns a pointer to the popped data or NULL if the stack is empty */item_t* pop_stack(stack *mystack){    if(mystack->size == 0) {        return NULL;    } else {        return &mystack->elements[--mystack->size];    }}/* Returns a pointer to the adjusted stack or NULL on failure */stack* resize_stack(stack *mystack, size_t capacity){    item_t *tmp = realloc(mystack->elements, sizeof(item_t) * capacity);    if(tmp == NULL)         return NULL;    mystack->elements = tmp;    mystack->capacity = capacity;    if(mystack->size > capacity)        mystack->size = capacity;    return mystack;}

main.c

#include "stacklib.h"int main(void){    stack *my_stack = create_stack(20);    item_t i;    for(i = 0; i < 20; i++) {        push_stack(my_stack, i);     }    resize_stack(my_stack, 10);    item_t res, *item ;    while((item = (item_t*)pop_stack(my_stack)) != NULL) {        res = *item;        printf("data: %d\n", res);    }    delete_stack(my_stack);    printf("last item popped was  %d\n", res);    return 0;}

Question 2

It looks very nice, code is clean and readable, not much else to change, I think...

This is more of a personal preference, not an universal recommendation: One of the things I like about C is having fine control over memory allocations, thus making it a very good choice for systems with limited memory or very rudimentary memory allocators. Because of that, I like to avoid allocating memory whenever possible, so I would not allocate eachstack instance on the heap withmalloc. Rather, let the user decide where to place it, be on the program stack, as a global or heap-allocated. To do that change, you'd just have to remove that allocation fromcreate_stack and one free fromdelete_stack, taking a pointer to an external object in create:

bool create_stack(stack* mystack, size_t capacity);

That might seem like a little thing, but forcing the user to dynamically allocate astack object when it doesn't have to is suboptimal and can be a problem on systems that don't rely on virtual memory and have fragmentation issues.

I strongly recommend that you usebooleans instead of error codes when a function has only two possible outcomes, success or failure. There's no universal agreement on the meaning of error codes, e.g. some return 0 on success, some return 1, some return -1 on error, and so on. This can be a major source of confusion. Use thebool type from<stdbool.h> whenever it makes sense.

Avoid repeating types in yourmalloc calls. E.g.:

item_t *elements_ptr = malloc(capacity*sizeof(item_t));

Can be refactored to:

item_t *elements_ptr = malloc(capacity * sizeof(*elements_ptr));

Notice that you can use the variable name in thesizeof expression, in this case dereferenced, since we want the size of the type itself, not the size of a pointer. Makes your life easier if you change the name of the type you are allocating; one less place to be updated.

Make sure to print errors tostderr. That's the traditional error output and allows users to filter errors from normal program output. Just replace theprintf call inhandle_error byfprintf(stderr).

Two last tips:

You might want to allow the users of your stack to provide their own error handler. You can achieve that withfunction pointers.

All functions receive pointers tostack, which could be passed in as null by error or due to some memory allocation failure. You canassert that they are not null to trap errors early and ease the debugging efforts.

glampert 17.3k4 gold badges31 silver badges89 bronze badges · Answer 1 · 2017-05-23 12:41:07Z

It looks very nice, code is clean and readable, not much else to change, I think...

This is more of a personal preference, not an universal recommendation: One of the things I like about C is having fine control over memory allocations, thus making it a very good choice for systems with limited memory or very rudimentary memory allocators. Because of that, I like to avoid allocating memory whenever possible, so I would not allocate eachstack instance on the heap withmalloc. Rather, let the user decide where to place it, be on the program stack, as a global or heap-allocated. To do that change, you'd just have to remove that allocation fromcreate_stack and one free fromdelete_stack, taking a pointer to an external object in create:

bool create_stack(stack* mystack, size_t capacity);

That might seem like a little thing, but forcing the user to dynamically allocate astack object when it doesn't have to is suboptimal and can be a problem on systems that don't rely on virtual memory and have fragmentation issues.

I strongly recommend that you usebooleans instead of error codes when a function has only two possible outcomes, success or failure. There's no universal agreement on the meaning of error codes, e.g. some return 0 on success, some return 1, some return -1 on error, and so on. This can be a major source of confusion. Use thebool type from<stdbool.h> whenever it makes sense.

Avoid repeating types in yourmalloc calls. E.g.:

item_t *elements_ptr = malloc(capacity*sizeof(item_t));

Can be refactored to:

item_t *elements_ptr = malloc(capacity * sizeof(*elements_ptr));

Notice that you can use the variable name in thesizeof expression, in this case dereferenced, since we want the size of the type itself, not the size of a pointer. Makes your life easier if you change the name of the type you are allocating; one less place to be updated.

Make sure to print errors tostderr. That's the traditional error output and allows users to filter errors from normal program output. Just replace theprintf call inhandle_error byfprintf(stderr).

Two last tips:

You might want to allow the users of your stack to provide their own error handler. You can achieve that withfunction pointers.

All functions receive pointers tostack, which could be passed in as null by error or due to some memory allocation failure. You canassert that they are not null to trap errors early and ease the debugging efforts.

Movatterモバイル変換

Stack Exchange Network

ADT stack with a dynamic array (revision 1)

stacklib.h

stacklib.c

main.c

1 Answer1

You mustlog in to answer this question.

Linked

Related

Hot Network Questions

Subscribe to RSS