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

Linked List Improvements in C

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

Problem

I'm trying to write a concise linked list implementation in C and so far it looks pretty good. I'd like some pointers (pun) on how I can improve what I've got in terms of naming, consistency, efficiency, and anything else that's relevant.

```
#include
#include

typedef struct node {
int data;
struct node *next;
}node_t;

int length(node_t *head){
int length = 0;
for(node_t *cur = head; cur; cur = cur->next)
length++;
return length;
}

void printList(node_t *head){
for(node_t *cur = head; cur; cur = cur->next)
printf("%d\n", cur->data);
}

void deleteList(node_t **head){
node_t current = head, *next;
while (current != NULL) {
next = current->next;
free(current);
current = next;
}
*head = NULL;
}

void deleteNode(node_t node, node_t head){
node_t *cur = head;
while (cur->next != node)
cur = cur->next;
cur->next = node->next;
free(node); node=NULL;
}

node_t getNode(int key, node_t head){
node_t *node = head;
while(node){
if (node->data == key){ return node; }
else { node = node->next; }
}
return NULL;
}

node_t insert(int key, node_t head){
node_t *cur = head;
while(cur->next)
cur = cur->next;
node_t *new = malloc(sizeof(node_t));
if (new == NULL){ exit(-1); }
new->data = key;
new->next = NULL;
cur->next = new;
return new;
}

node_t *createList(int key) {
node_t *head = malloc(sizeof(node_t));
if (head == NULL){ exit(-1); }
head->data = key;
head->next = NULL;
return head;
}

int main(){
node_t *head = createList(1);
insert(2, head);
node_t *nodeA = insert(3, head);
insert(4, head);
printf("1) Printing list:\n");
printList(head);
node_t *nodeB = getNode(4, head);
deleteNode(nodeA, head);
deleteNode(nodeB, head);
printf("2) Printing list:\n");
printList(head);
deleteList(&head);
printf("3) Printing list:\n");
pri

Solution

Bug: can't delete head node

Your delete_node() function doesn't work if the node you want to delete is the head of the list. In fact, the function signature can't work:

void deleteNode(node_t *node, node_t *head);


Since the head pointer can change if you delete it, your signature has to be one of these two instead;

void deleteNode(node_t *node, node_t **head);
node_t *deleteNode(node_t *node, node_t *head);


Insert should be renamed

Since your "insert" function adds to the end of the list, I think it should be called append() instead.

Code Snippets

void deleteNode(node_t *node, node_t *head);
void deleteNode(node_t *node, node_t **head);
node_t *deleteNode(node_t *node, node_t *head);

Context

StackExchange Code Review Q#155478, answer score: 3

Revisions (0)

No revisions yet.