4
\$\begingroup\$

I have been trying to come up a generic C linked list to store data as it comes and later sort them to use it for various purpose. Please have a look at the code below an provide me your review comments. This code compiles and runs in Windows and Linux.

  1. Naming

  2. Design

  3. Quality

  4. Usability

you may run the main.c for example output.

node.h

#ifndef NODE_H
#define NODE_H

typedef struct node Node;

struct node{
    void* data;
    Node* next;
    Node* previous;
};

#endif  //NODE_H

sll.h

#ifndef SLL_H
#define SLL_H
#include "node.h"

typedef struct sllist SLList;

struct sllist {
    Node*   head;
    Node*   tail;
    int     _num_nodes;
    int     (*compare)(const void* const, const void* const);
    int     (*compare_sort)(const void* const, const void* const);
    void    (*copy)(const void*, const void*);
    void    (*display)(const void*);
    void*   (*allocate)(const void* const);
    void    (*deallocate)(void*);
};

int    remove_data(SLList* list, void* data);               // removes the first occurrence of the data in the list
int    remove_at_index(SLList* list, int index);            // removes node at the given index
Node*  add_data(SLList* list, void* data);                  // adds at the end and returns the node
Node*  add_at_index(SLList* list, void* data, int index);   // adds at the index, if index is zero, it is add at the front/head
Node*  get(SLList* l, int index);
Node*  set(SLList* list, void* data, int index);            // modifies the value at the index
Node*  find(SLList* list, void* data);
int    index_of(SLList* list, void* data);
int    length(SLList* list);
int    is_empty(SLList* list);
void** sort_data(SLList* list);
void   display(SLList* list);
void   free_list(SLList**);
Node*  _find(SLList* list, void* data, int* _index);
void** _to_array(SLList* list);

#endif  //SLL_H

sll.c

#include <stdio.h>
#include <assert.h>
#include <stdlib.h>
#include "sll.h" 

int remove_data(SLList* list, void* data)
{
    list->_num_nodes = list->_num_nodes - 1;
    return 0;
}

int remove_at_index(SLList* list, int index)
{
    list->_num_nodes = list->_num_nodes - 1;
    return 0;
}

Node* add_data(SLList* list, void* data)
{
    Node* new_node = NULL;
    assert(list != NULL && data != NULL);
    new_node = (Node*)malloc(sizeof(Node));
    new_node->data = list->allocate(data);
    //new_node->data = (void*)malloc(list->size(data));
    list->copy(new_node->data, data);
    new_node->next = NULL;
    if (list->tail == NULL)
    {
        list->head = list->tail = new_node;
    }
    else
    {
        list->tail->next = new_node;
        list->tail = new_node;
    }
    list->_num_nodes = list->_num_nodes + 1;
    return new_node;
}

Node* add_at_index(SLList* list, void* data, int index)
{
    Node* ret_value = NULL;
    list->_num_nodes = list->_num_nodes + 1;
    return ret_value;
}

Node* get(SLList* list, int index)
{
    Node* ret_value = NULL;
    return ret_value;
}

Node* find(SLList* list, void* data)
{
    Node* ret_value = NULL;
    return ret_value;
}

Node* set(SLList* list, void* data, int index)
{
    Node* ret_value = NULL;
    return ret_value;
}

int index_of(SLList* list, void* data)
{
    return 0;
} 

int length(SLList* list)
{
    return list->_num_nodes;
}

int is_empty(SLList* list)
{
    return 0;
}

void** sort_data(SLList* list)
{
    void **_array = NULL;
    assert(list != NULL && list->compare != NULL);
    _array = _to_array(list);
    qsort(_array, length(list), sizeof(*_array), list->compare_sort);
    return _array;
} 

void display(SLList* list)
{
    Node* cur = NULL;
    assert(list != NULL && list->display != NULL);
    cur = list->head;
    while (cur != NULL)
    {
        list->display(cur->data);
        cur = cur->next;
    }
}

void free_list(SLList** list)
{
    Node* cur = NULL;
    Node* temp = NULL;
    assert(list != NULL && *list != NULL);
    cur = (*list)->head;
    while (cur != NULL)
    {
        temp = cur;
        cur = cur->next;
        (*list)->deallocate(temp);      
    }
    (*list)->head = NULL;
    (*list)->tail = NULL;
    free(*list);
    *list = NULL;
}

Node* _find(SLList* list, void* data, int* index)
{
    Node* ret_value = NULL;
    *index = -1;
    assert(list != NULL && list->compare != NULL && data != NULL);

    return ret_value;
} 

void** _to_array(SLList* list)
{
    void** _array = NULL;
    Node* cur = NULL;
    int _index = 0;
    assert(list != NULL && list->copy != NULL);
    _array = (void**)malloc(list->_num_nodes*sizeof(*_array));
    _index = 0;
    cur = list->head;
    while (cur != NULL)
    {
        _array[_index] = list->allocate(cur->data);
        list->copy(_array[_index], cur->data);
        cur = cur->next;
        ++_index;
    }
    return _array;
}

sll_char.h

#ifndef SLL_CHAR_H
#define SLL_CHAR_H

#include "sll.h"

void    init_sll_char(SLList**);
int     compare_char(const void* const, const void* const);
int     compare_sort_char(const void* const, const void* const);
void    copy_char(const void*, const void*);
void    display_char(const void*);
void*   allocate_char(const void* const);
void    deallocate_char(void*);

#endif  //SLL_CHAR_H

sll_char.c

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <assert.h>

#include "sll_char.h"

void init_sll_char(SLList** sllist)
{
    // TODO we are not allocating any memory for the underying element yet
    *sllist = (SLList*)malloc(sizeof(SLList));
    (*sllist)->head = NULL;
    (*sllist)->tail = NULL;
    (*sllist)->_num_nodes = 0;
    (*sllist)->compare = compare_char;
    (*sllist)->compare_sort = compare_sort_char;
    (*sllist)->copy = copy_char;
    (*sllist)->display = display_char;
    (*sllist)->allocate = allocate_char;
    (*sllist)->deallocate = deallocate_char;
}

int compare_char(const void* const lhs, const void* const rhs)
{
    const char* const clhs = (const char* const)lhs;
    const char* const crhs = (const char* const)rhs;
    return strcmp(clhs, crhs);
}

int compare_sort_char(const void* const lhs, const void* const rhs)
{
    const char *clhs = *(const char**)lhs;
    const char *crhs = *(const char**)rhs;
    return strcmp(clhs, crhs);
} 

void copy_char(const void* lhs, const void* rhs)
{
    char* const clhs = (char* const)lhs;
    const char* const crhs = (const char*)rhs;
    strcpy(clhs, crhs);
} 

void display_char(const void* const value)
{
    const char* const val_ptr = (const char*)value;
    printf(" Value: %s\n", val_ptr);
}

void* allocate_char(const void* const data)
{
    char *_data = (char*)malloc((strlen((char*)data)+1)*sizeof(char));  
    return (void*)_data;
}

void deallocate_char(void* data)
{
    free(data);
    data = NULL;
}

sll_int.h

#ifndef SLL_INT_H
#define SLL_INT_H

#include "sll.h"

void    init_sll_int(SLList**);
int     compare_int(const void* const, const void* const);
int     compare_sort_int(const void* const, const void* const);
void    copy_int(const void*, const void*);
void    display_int(const void*);
void*   allocate_int(const void* const);
void    deallocate_int(void*);

#endif  //SLL_INT_H

sll_sllchar.h

#ifndef SLL_SLL_CHAR_H
#define SLL_SLL_CHAR_H

#include "sll.h"

typedef struct sllchar SLLChar;

struct sllchar {
    char* data;
    int   id;
    SLList* list;
};

void    init_sll_sllchar(SLList**);
int     compare_sllchar(const void* const, const void* const);
int     compare_sort_sllchar(const void* const, const void* const);
void    copy_sllchar(const void*, const void*);
void    display_sllchar(const void*);
void*   allocate_sllchar(const void* const);
void    deallocate_sllchar(void*);

#endif  //SLL_SLL_CHAR_H

sll_sllchar.c

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <assert.h>

#include "sll_sllchar.h"

void init_sll_sllchar(SLList** sllist)
{
    // TODO we are not allocating any memory for the underying element yet
    *sllist = (SLList*)malloc(sizeof(SLList));
    (*sllist)->head = NULL;
    (*sllist)->tail = NULL;
    (*sllist)->_num_nodes = 0;
    (*sllist)->compare = compare_sllchar;
    (*sllist)->compare_sort = compare_sort_sllchar;
    (*sllist)->copy = copy_sllchar;
    (*sllist)->display = display_sllchar;
    (*sllist)->allocate = allocate_sllchar;
    (*sllist)->deallocate = deallocate_sllchar;
}

int compare_sllchar(const void* const lhs, const void* const rhs)
{
    const char* const clhs = (const char* const)lhs;
    const char* const crhs = (const char* const)rhs;
    return strcmp(clhs, crhs);
}

int compare_sort_sllchar(const void* const lhs, const void* const rhs)
{
    SLLChar* slhs = *((SLLChar**)lhs);
    SLLChar* srhs = *((SLLChar**)rhs);
    return strcmp(slhs->data, srhs->data);
} 

void copy_sllchar(const void* lhs, const void* rhs)
{
    SLLChar* llhs = (SLLChar*)lhs;
    SLLChar* lrhs = (SLLChar*)rhs;    
    strcpy(llhs->data, lrhs->data);
    llhs->id = lrhs->id;
    llhs->list = lrhs->list;
} 

void display_sllchar(const void* const value)
{
    const SLLChar* const val_ptr = (const SLLChar*)value;
    printf(" Data: %s, id: %d\n", val_ptr->data, val_ptr->id);
    display(val_ptr->list);
}

void* allocate_sllchar(const void* const data)
{
    SLLChar *s = NULL;
    SLLChar* _data = (SLLChar*)data;
    assert(_data != NULL && _data->data != NULL);
    s = (SLLChar*)malloc(sizeof(SLLChar));
    assert(s != NULL);
    s->data = (char*)malloc((strlen(_data->data)+1)*sizeof(char));
    strcpy(s->data, "");
    s->id = 0;
    s->list = NULL;
    return (void*)s;
}

void deallocate_sllchar(void* data)
{
    SLLChar* _data = (SLLChar*)data;
    assert(_data != NULL && _data->data != NULL);
    free(_data->data); _data->data = NULL;
    _data->list = NULL; // we will set this to NULL, will not free the list as we have not allocated memory for this
    free(_data); _data = NULL;
}

sll_student.h

#ifndef SLL_STUDENT_H
#define SLL_STUDENT_H

#include "sll.h"

typedef struct student Student;

struct student {
    char* name;
    int   id;
};

void    init_sll_student(SLList**);
int     compare_student(const void* const, const void* const);
int     compare_sort_student(const void* const, const void* const);
void    copy_student(const void*, const void*);
void    display_student(const void*);
void*   allocate_student(const void* const);
void    deallocate_student(void*);

#endif  //SLL_STUDENT_H

sll_student.c

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <assert.h>

#include "sll_student.h"

void init_sll_student(SLList** sllist)
{
    // TODO we are not allocating any memory for the underying element yet
    *sllist = (SLList*)malloc(sizeof(SLList));
    (*sllist)->head = NULL;
    (*sllist)->tail = NULL;
    (*sllist)->_num_nodes = 0;
    (*sllist)->compare = compare_student;
    (*sllist)->compare_sort = compare_sort_student;
    //(*sllist)->size = size_student;
    (*sllist)->copy = copy_student;
    (*sllist)->display = display_student;
    (*sllist)->allocate = allocate_student;
    (*sllist)->deallocate = deallocate_student;
}

int compare_student(const void* const lhs, const void* const rhs)
{
    const char* const clhs = (const char* const)lhs;
    const char* const crhs = (const char* const)rhs;
    return strcmp(clhs, crhs);
}

int compare_sort_student(const void* const lhs, const void* const rhs)
{
    Student* slhs = *((Student**)lhs);
    Student* srhs = *((Student**)rhs);
    return strcmp(slhs->name, srhs->name);
} 

void copy_student(const void* lhs, const void* rhs)
{
    Student* llhs = (Student*)lhs;
    Student* lrhs = (Student*)rhs;
    llhs->id = lrhs->id;
    //llhs->name = (char*)malloc((strlen(lrhs->name)+1)*sizeof(char));
    strcpy(llhs->name, lrhs->name);
} 

void display_student(const void* const value)
{
    const Student* const val_ptr = (const Student*)value;
    printf(" Student name: %s, id: %d\n", val_ptr->name, val_ptr->id);
}

void* allocate_student(const void* const data)
{
    Student *s = NULL;
    Student* _data = (Student*)data;
    assert(_data != NULL && _data->name != NULL);
    s = (Student*)malloc(sizeof(Student));
    assert(s != NULL);
    s->id = 0;
    s->name = (char*)malloc((strlen(_data->name)+1)*sizeof(char));
    strcpy(s->name, "");
    return (void*)s;
}

void deallocate_student(void* data)
{
    Student* _data = (Student*)data;
    assert(_data != NULL && _data->name != NULL);
    free(_data->name); _data->name = NULL;
    free(_data); _data = NULL;
}

main.c

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <assert.h>

#include "sll.h"
#include "sll_int.h"
#include "sll_char.h"
#include "sll_student.h"
#include "sll_sllchar.h"

int main()
{   
    int indx = 0;
    int indx_dept = 0, indx_class = 0, indx_student = 0;

    SLList* intlistptr1 = NULL;
    void**  int_array_1 = NULL;
    int     i = 0;

    SLList* charlistptr1 = NULL;
    void**  chararray1 = NULL;
    char*   str = NULL;

    SLList* studentlistptr1 = NULL;
    void**  student_array_1 = NULL;
    Student* s1 = NULL;   

    // University (is a list and data = "ABC University", list = list_dept)
    //     |-- Department1 (is a node and data = "Department1", list = list_class_dept1)
    //             |-- Class11 (is a node and data = "Class11", list = student_list)
    //                   |- Student-111
    //                   |- Student-112
    //                   |- Student-113
    //             |-- Class12 (is a node and data = "Class21", list = student_list)
    //                   |- Student-121
    //                   |- Student-122
    //                   |- Student-123
    //             |-- Class13
    //                   |- Student-131
    //                   |- Student-232
    //                   |- Student-333
    //     |-- Department2 (is a node and data = "Department2", list = list_class_dept2)
    //             |-- Class21
    //                   |- Student-211
    //                   |- Student-212
    //                   |- Student-213                
    //             |-- Class22
    //                   |- Student-221
    //                   |- Student-222
    //                   |- Student-223
    //             |-- Class23
    //                   |- Student-231
    //                   |- Student-232
    //                   |- Student-233
    //     |-- Department3 (is a node and data = "Department3", list = list_class_dept3)
    //             |-- Class31
    //                   |- Student-311
    //                   |- Student-312
    //                   |- Student-313                
    //             |-- Class32
    //                   |- Student-321
    //                   |- Student-322
    //                   |- Student-323
    //             |-- Class33
    //                   |- Student-331
    //                   |- Student-332
    //                   |- Student-333

    // University List
    SLList* list_univ = NULL;
    SLLChar* node_univ = NULL;
    SLList* list_dept= NULL;
    SLLChar* node_dept1 = NULL;
    SLList* list_class_dept1 = NULL;
    SLLChar* node_class11 = NULL;
    SLLChar* node_class12 = NULL;
    SLLChar* node_class13 = NULL;
    SLLChar* node_dept2 = NULL;
    SLList* list_class_dept2 = NULL;
    SLLChar* node_class21 = NULL;
    SLLChar* node_class22 = NULL;
    SLLChar* node_class23 = NULL;
    SLLChar* node_dept3 = NULL;
    SLList* list_class_dept3 = NULL;
    SLLChar* node_class31 = NULL;
    SLLChar* node_class32 = NULL;
    SLLChar* node_class33 = NULL;
    void** array_dept = NULL;
    void** array_class = NULL;
    void** array_student = NULL;
    SLList* temp_list =  NULL;
    SLLChar* temp_node = NULL;

    init_sll_int(&intlistptr1);
    i = 51; 
    add_data(intlistptr1, &(i));
    i = 63;   
    add_data(intlistptr1, &(i));
    i = 10; 
    add_data(intlistptr1, &(i));
    i = 11; 
    add_data(intlistptr1, &(i));
    i = 104; 
    add_data(intlistptr1, &(i));
    i = 61; 
    add_data(intlistptr1, &(i));
    printf("Before sort:\n");
    display(intlistptr1);   
    int_array_1 = sort_data(intlistptr1);
    printf("After sort:\n");
    for (indx = 0; indx < length(intlistptr1); ++indx)
    {
        printf("  Value: %d\n", *((int*)int_array_1[indx]));
    }

    str = (char*)malloc(11*sizeof(char));
    init_sll_char(&charlistptr1);
    strcpy(str, "World");
    add_data(charlistptr1, str);
    strcpy(str, "Hello");
    add_data(charlistptr1, str);
    strcpy(str, "xyz");
    add_data(charlistptr1, str);
    strcpy(str, "pqr");
    add_data(charlistptr1, str);
    strcpy(str, "jrk");
    add_data(charlistptr1, str);
    strcpy(str, "abc");
    add_data(charlistptr1, str);
    printf("Before sort:\n");
    display(charlistptr1);
    chararray1 = sort_data(charlistptr1);
    printf("After sort:\n");
    for (indx = 0; indx < length(charlistptr1); ++indx)
    {
        printf("  Value: %s\n", (char*)(chararray1[indx]));
    }

    init_sll_student(&studentlistptr1);
    s1 = (Student*)malloc(sizeof(Student));
    s1->name = (char*)malloc(20*sizeof(char));
    s1->id = 21; strcpy(s1->name, "Tom");
    add_data(studentlistptr1, s1);
    s1->id = 31; strcpy(s1->name, "Harry");
    add_data(studentlistptr1, s1);
    s1->id = 26; strcpy(s1->name, "Sam");
    add_data(studentlistptr1, s1);
    s1->id = 23; strcpy(s1->name, "Ram");
    add_data(studentlistptr1, s1);
    s1->id = 18; strcpy(s1->name, "Xiu");
    add_data(studentlistptr1, s1);
    s1->id = 40; strcpy(s1->name, "Cam");
    add_data(studentlistptr1, s1);  
    printf("Before sort:\n");
    display(studentlistptr1);
    student_array_1 = sort_data(studentlistptr1);
    printf("After sort:\n");
    for (indx = 0; indx < length(studentlistptr1); ++indx)
    {
        printf("  Student name: %s, id: %d\n", ((Student*)(student_array_1[indx]))->name, 
            ((Student*)(student_array_1[indx]))->id);
    }

    //
    init_sll_sllchar(&list_class_dept1);
    node_class12 = (SLLChar*)malloc(sizeof(SLLChar));
    node_class12->data = (char*)malloc(20*sizeof(char));
    strcpy(node_class12->data, "Class12");
    node_class12->id = 12; 
    node_class12->list = studentlistptr1;   
    add_data(list_class_dept1, node_class12);

    node_class13 = (SLLChar*)malloc(sizeof(SLLChar));
    node_class13->data = (char*)malloc(20*sizeof(char));
    strcpy(node_class13->data, "Class13");
    node_class13->id = 13; 
    node_class13->list = studentlistptr1;   
    add_data(list_class_dept1, node_class13);

    node_class11 = (SLLChar*)malloc(sizeof(SLLChar));
    node_class11->data = (char*)malloc(20*sizeof(char));
    strcpy(node_class11->data, "Class11");
    node_class11->id = 11; 
    node_class11->list = studentlistptr1;   
    add_data(list_class_dept1, node_class11);

    //
    init_sll_sllchar(&list_class_dept2);
    node_class22 = (SLLChar*)malloc(sizeof(SLLChar));
    node_class22->data = (char*)malloc(20*sizeof(char));
    strcpy(node_class22->data, "Class22");
    node_class22->id = 22; 
    node_class22->list = studentlistptr1;   
    add_data(list_class_dept2, node_class22);

    node_class23 = (SLLChar*)malloc(sizeof(SLLChar));
    node_class23->data = (char*)malloc(20*sizeof(char));
    strcpy(node_class23->data, "Class23");
    node_class23->id = 23; 
    node_class23->list = studentlistptr1;   
    add_data(list_class_dept2, node_class23);

    node_class21 = (SLLChar*)malloc(sizeof(SLLChar));
    node_class21->data = (char*)malloc(20*sizeof(char));
    strcpy(node_class21->data, "Class21");
    node_class21->id = 21; 
    node_class21->list = studentlistptr1;   
    add_data(list_class_dept2, node_class21);

    //
    init_sll_sllchar(&list_class_dept3);
    node_class32 = (SLLChar*)malloc(sizeof(SLLChar));
    node_class32->data = (char*)malloc(20*sizeof(char));
    strcpy(node_class32->data, "Class32");
    node_class32->id = 32; 
    node_class32->list = studentlistptr1;   
    add_data(list_class_dept3, node_class32);

    node_class33 = (SLLChar*)malloc(sizeof(SLLChar));
    node_class33->data = (char*)malloc(20*sizeof(char));
    strcpy(node_class33->data, "Class33");
    node_class33->id = 33; 
    node_class33->list = studentlistptr1;   
    add_data(list_class_dept3, node_class33);

    node_class31 = (SLLChar*)malloc(sizeof(SLLChar));
    node_class31->data = (char*)malloc(20*sizeof(char));
    strcpy(node_class31->data, "Class31");
    node_class31->id = 11; 
    node_class31->list = studentlistptr1;   
    add_data(list_class_dept3, node_class31);

    //list for Dept
    init_sll_sllchar(&list_dept);
    // node for 2
    node_dept2 = (SLLChar*)malloc(sizeof(SLLChar));
    node_dept2->data = (char*)malloc(20*sizeof(char));
    strcpy(node_dept2->data, "Department2");
    node_dept2->id = 2; 
    node_dept2->list = list_class_dept2;    
    add_data(list_dept, node_dept2);
    // node for 3
    node_dept3 = (SLLChar*)malloc(sizeof(SLLChar));
    node_dept3->data = (char*)malloc(20*sizeof(char));
    strcpy(node_dept3->data, "Department3");
    node_dept3->id = 2; 
    node_dept3->list = list_class_dept3;    
    add_data(list_dept, node_dept3);
    // node for Department1
    node_dept1 = (SLLChar*)malloc(sizeof(SLLChar));
    node_dept1->data = (char*)malloc(20*sizeof(char));
    strcpy(node_dept1->data, "Department1");
    node_dept1->id = 1; 
    node_dept1->list = list_class_dept1;    
    add_data(list_dept, node_dept1);

    init_sll_sllchar(&list_univ);
    node_univ = (SLLChar*)malloc(sizeof(SLLChar));
    node_univ->data = (char*)malloc(20*sizeof(char));
    strcpy(node_univ->data, "University");
    node_univ->id = 100; 
    node_univ->list = list_dept;    
    add_data(list_univ, node_univ);
    display(list_univ);

    printf("University After sort:\n");    
    temp_list = ((SLLChar*)(list_univ->head->data))->list;
    assert(temp_list != NULL);
    array_dept = sort_data(temp_list); 
    for (indx_dept = 0; indx_dept < length(temp_list); ++indx_dept)
    {
        printf("  Dept: %s\n", ((SLLChar*)(array_dept[indx_dept]))->data);
        array_class = sort_data(((SLLChar*)(array_dept[indx_dept]))->list); 
        for (indx_class = 0; indx_class < length(((SLLChar*)(array_dept[indx_dept]))->list); ++indx_class)
        {
            printf("    Class: %s\n", ((SLLChar*)(array_class[indx_class]))->data);
            array_student = sort_data(((SLLChar*)(array_class[indx_class]))->list); 
            for (indx_student = 0; indx_student < length(((SLLChar*)(array_class[indx_class]))->list); ++indx_student)
            {
                printf("      Student: %s\n", ((Student*)(array_student[indx_student]))->name);                
            }
        }
    }

    //free_list(&intlistptr1);
    //free_list(&charlistptr1);
    //free_list(&studentlistptr1);
    printf("End");
    return 0;
}
\$\endgroup\$
4
  • 5
    \$\begingroup\$ Nice effort. Though one remark: SLL sounds like single-linked-list, and you have double-linked-list with next and previous links. As DLL has its associations on Windows, maybe a fuller name would be better: dllist.h or such. \$\endgroup\$
    – Joop Eggen
    Commented Dec 13, 2019 at 12:43
  • \$\begingroup\$ I would expect something like a relational database for your example, not a linked-list, since eg one student could be in multiple classes. \$\endgroup\$
    – Neil
    Commented Dec 13, 2019 at 20:31
  • \$\begingroup\$ I didn't take enough time to add different students in each class. In a real scenario, the students in each class will have their own list. \$\endgroup\$
    – Amit
    Commented Dec 13, 2019 at 20:50
  • \$\begingroup\$ I took a note of the suggestion from Joop and will incorporate it in my solution. \$\endgroup\$
    – Amit
    Commented Dec 13, 2019 at 20:57

2 Answers 2

5
\$\begingroup\$

Overall, you might want to consider using private encapsulation with opaque type instead of public structs, in order to block the users from accessing private parts of the data.

  • Avoid casting the result of malloc since that only adds pointless clutter and could cause subtle bugs on old C90 compilers. I'd recommend using new_node = malloc(sizeof *new_node); instead.
  • Similarly, you don't need to cast when doing to void pointers, like return (void*)s;. The most common reason why people do this is because they compile C code with a C++ compiler, which is a plain bad idea.
  • Multiplying something with sizeof(char) is pointless clutter, since sizeof(char) is guaranteed to always be 1 and unlike the size for other types, it can never be anything else.
  • Don't prefix identifiers with underscore since that might collide with identifiers reserved for the compiler or the standard library.
  • Don't use const void* const lhs form for parameters to comparison functions. The canonical comparison functor in C uses the format of the bsearch/qsort functors:

    int func (const void* obj1, const void* obj2)

    It doesn't even make sense to const-qualify local function variables, since they are just a local variable to begin with and the caller couldn't care less about what your function does with that local variable internally.

  • const char *clhs = *(const char**)lhs; is a non-compatible pointer conversion and as such very fishy. Overall, pointer-to-pointer cannot be converted to a pointer. And void** is not a generic pointer type like void*. And to cast away const is even more questionable since it invokes undefined behavior, like you do in SLLChar* slhs = *((SLLChar**)lhs);.

    As a rule of thumb, whenever you find yourself in need of an explicit cast when dealing with void pointers, the root of the problem lies elsewhere. A properly written generic C code with void pointers shouldn't need casts anywhere. This is by far the biggest problem with your code.

    I'd advise removing all casts from this program, then get to the bottom with each single pointer type warning you get from the compiler and fix the actual root cause.

  • Declare for loop iterators inside the for loop. This code won't compile in C90 anyway, so there is no reason not to do this.

  • To null terminate a string, use s->data[0] = '\0'; rather than strcpy(s->data, "");. Clearer and faster.

  • My preferred style is to place all library includes in the .h file, not the .c file. This documents all dependencies of your code to the caller, who needs to know them. They shouldn't need to dig through the .c file when the program won't link for whatever reason.

\$\endgroup\$
1
  • \$\begingroup\$ const char clhs = *(const char*)lhs; is a non-compatible pointer conversion and as such very fishy. Overall, pointer-to-pointer cannot be converted to a pointer. And Thanks for your valuable suggestions. void** is not a generic pointer type like void*. And to cast away const is even more questionable since it invokes undefined behavior, like you do in SLLChar* slhs = ((SLLChar*)lhs); I agree with this and didn't want to use it in this way, but was forced to avoid run time error in qsort. Any suggestions for that? \$\endgroup\$
    – Amit
    Commented Dec 13, 2019 at 20:55
2
\$\begingroup\$

to come up a generic C linked list to store data

Short-coming in generic.

Code requires data to not be NULL due to assert(... data != NULL);

The legal values in void *data should be determined in the member functions compare, copy, allocate, etc., not in add_data(), etc.

Name space

Could uses global names like struct node, get(), find(), is_empty(), ... This is certain to collide with other code.

Instead consider all sll items to begin with SSL_ like struct SSL_node, SSL_get(), SSL_find(), SSL_is_empty().

Use const *

Functions like is_empty(SLList* list); which do not alter * list could use is_empty(const SLList* list) to 1) expand functionality (Allows const calls), 2) convey functionality better 3) Allow for some optimizations not otherwise recognized by a compiler.

Do not return internal workings

Node *find(SLList* list, void* data); should return the data, not the internal structure. Calling code has no business working with Node members.

Hide information

#include "node.h" is not needed in sll.h as struct sllist { Node* head; Node* tail; int _num_nodes; ... }; is not needed there. Hide the implementation from the caller. Example: stdio functions have FILE *, yet good user code does not "know" FILE.

Search on key

Instead of only find(SLList* list, void* data);, consider another search based on a key like find_key(SLList* list, void* key, ...); that uses some the passed in parameters to form a search. Perhaps pass in a key, data compare function?

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.