patterncModerate
C Doubly Linked List
Viewed 0 times
listdoublylinked
Problem
It works, but it seems a bit long winded. I'd like some critiquing.
#include
#include
struct node
{
int data;
struct node * previous;
struct node * next;
};
struct linked_list
{
struct node * first;
struct node * last;
};
struct linked_list * init_list(struct node * node)
{
struct linked_list * ret = malloc(sizeof(struct linked_list));
ret->first = node;
ret->last = node;
return ret;
}
struct node * create_node(int data)
{
struct node * ret = malloc(sizeof(struct node));
ret->data = data;
ret->previous = NULL;
ret->next = NULL;
return ret;
}
void destroy_list(struct linked_list * list)
{
struct node * node = list->first, *next = NULL;
while(node != NULL)
{
next = node->next;
free(node);
node = next;
}
free(list);
}
void push_front(struct linked_list * list, struct node * node)
{
if(list->first != NULL)
{
node->next = list->first;
list->first->previous = node;
}
else list->last = node;
list->first = node;
}
void push_back(struct linked_list * list, struct node * node)
{
if(list->last != NULL)
{
node->previous = list->last;
list->last->next = node;
list->last = node;
}
else push_front(list, node);
}
void insert_after(struct node * node, struct node * to_add)
{
to_add->previous = node;
to_add->next = node->next;
node->next = to_add;
}
void remove_node(struct node * node)
{
node->previous->next = node->next;
node->next->previous = node->previous;
free(node);
}Solution
As mentioned in the comments this code is not long winded at all. It is very clean in general. Too clean in fact as you will see below.
-
You cannot create an empty linked list with your
-
I personally like symmetry in linked list code. You can always create the dual of one function by swapping all prevs with nexts and all heads with tails. I don't think your
-
-
-
As simply a style choice I would rename
-
I am ignoring that you never check for your pointers being
Edit:
This code actually can produce an empty list by passing
-
You cannot create an empty linked list with your
init_list. However, in other parts of your code you assume list may be empty. It is useful to be able to create an empty linked list since you may not know what you want your first node to be in advance. I'd suggest changing init_list.-
I personally like symmetry in linked list code. You can always create the dual of one function by swapping all prevs with nexts and all heads with tails. I don't think your
push_back code should call push_front. You can handle the else condition just like you did in push_front except by swapping variable names.-
insert_after is incomplete. Remember this is a doubly linked list. Also the function doesn't handle the case when you insert after the current list->last. You need to add the linked list to the function arguments.-
remove_node does not handle all cases. What if node was the head of the list? What if it was the tail of the list? What if it was both the head and the tail? You don't actually have to write three different cases to handle this but you can.-
As simply a style choice I would rename
previous to prev-
I am ignoring that you never check for your pointers being
NULLEdit:
This code actually can produce an empty list by passing
NULL in as an argument to init_listContext
StackExchange Code Review Q#82492, answer score: 11
Revisions (0)
No revisions yet.