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

Linked deque implementation in C

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

Problem

I wrote a deque implementation in C for storing char *. It uses a doubly linked list, but I'm calling it a deque since it can only push/pop from the head and tail of the list.

I don't have much experience writing C (I'm more of a Python guy) so please let me know what can be improved!

I am already aware of the following:

  • there are no comments (I've tried to use self-explanatory names)



  • the testing isn't particularly rigorous



linkedlist.h

#ifndef _LINKEDLIST_H
#define _LINKEDLIST_H
#include 

struct ll_node {
    struct ll_node *prev;
    struct ll_node *next;
    char           *data;
};

struct linkedlist {
    struct ll_node *head;
    struct ll_node *tail;
    size_t         length;
};

void ll_node_init(struct ll_node *node, char *data);
void ll_init(struct linkedlist *ll);
void ll_destroy(struct linkedlist *ll);
void ll_push_head(struct linkedlist *ll, char *data);
void ll_push_tail(struct linkedlist *ll, char *data);
char *ll_pop_head(struct linkedlist *ll);
char *ll_pop_tail(struct linkedlist *ll);

#endif


linkedlist.c

```
#include
#include "linkedlist.h"

void ll_node_init(struct ll_node node, char data) {
node->prev = NULL;
node->next = NULL;
node->data = data;
}

void ll_init(struct linkedlist *ll) {
ll->head = NULL;
ll->tail = NULL;
ll->length = 0;
}

void ll_destroy(struct linkedlist *ll) {
struct ll_node *node = ll->head;
struct ll_node *next;

while(node != NULL) {
next = node->next;
free(node);
node = next;
}
}

void ll_push_head(struct linkedlist ll, char data) {
struct ll_node *node = malloc(sizeof(struct ll_node));
ll_node_init(node, data);

if(ll->head != NULL) {
ll->head->prev = node;
}

if(ll->tail == NULL) {
ll->tail = node;
}

node->next = ll->head;
ll->head = node;
ll->length++;
}

void ll_push_tail(struct linkedlist ll, char data) {
struct ll_node *node = malloc(sizeof(struct ll_node));
ll_no

Solution

Looks good. Few notes:

-
There is no reason to #include in both linkedlist.h and linkedlist.c. Pick one (I recommend linkedlist.c).

-
Allocation of a struct llnode instance definitely belongs to ll_node_init, and its signature should be

static struct ll_node * ll_node_init(char * data);


Notice static: there is no reason to expose struct ll_node to the client.

-
Maintain your invariants. It is clear that if (ll_head != NULL) then ll_tail != NULL, and vice versa. For example, push_head should have

if (ll->head != NULL) {
        assert(ll->tail != NULL);
        ll->head->prev = node;
    } else {
        assert(ll->tail == NULL);
        ll->tail = node;
    }


-
The code doesn't really care what kind of data it queues. You may consider making the structure more generic with void * data.

Code Snippets

static struct ll_node * ll_node_init(char * data);
if (ll->head != NULL) {
        assert(ll->tail != NULL);
        ll->head->prev = node;
    } else {
        assert(ll->tail == NULL);
        ll->tail = node;
    }

Context

StackExchange Code Review Q#156902, answer score: 2

Revisions (0)

No revisions yet.