I have seen many list implementations in C in this site; I know its been asked many times. I need some advice regarding:
- Quality of my code, especially my list library.
- How to handle errors in main (should every error be handled in
main()
?). - Can I handle errors within functions by calling
exit (EXIT_FAILURE)
? Some say it is bad practice since they have side effects. - Allocation errors, Read errors, function operation errors (how to handle?).
- In case of errors, how should I release memory? There may be multiple lists, nodes.
- Should I provide a message within each function in my library or give return values? (I like to give freedom to programmer to give message; is that bad?)
- I have
typedef
ed list and allocated structure in heap; is it okay?
I have tried to explicitly check errors by if
/else
conditionals but they began to cluster too much, leading to bad code.
list.h
#ifndef LIST_H
#define LIST_H
#ifdef __cplusplus
extern "C" {
#endif
#include <stdio.h>
#define ALLOC_FAIL NULL
typedef struct ListNode{
int data;
struct ListNode* next;
}node;
typedef struct LinkedList{
node* head;
node* tail;
size_t size;
}list;
list* createList(void); // allocation operations returns list or node //
node* getNode(int data); // else returns ALLOC_FAIL on failure //
node* insertAtFront(list* l,node* newnode);
node* insertAtLast(list*l,node* newnode);
node* nodeAt(list* l,size_t position); // node operations returns the //
node* insertNode(list* l,node* newnode,node* current); //inserted node upon success else NULL //
node* insertBefore(list*l,node* newnode,int searchData);
node* insertAfter(list*l,node* newnode,int searchData);
node* insertAtIndex(list* l,node* newnode,size_t index);
void printList(list* l);
void* destroyList(list* l); // returns NULL to clear the list variable //
#ifdef __cplusplus
}
#endif
#endif // LIST_H
list.c
#include <stdlib.h>
#include <stdio.h>
#include "list.h"
// All functions expect allocated memory ,it is up to the programmer to handle memory errors or leaks//
list* createList(void)
{
list* l = malloc(sizeof(list));
if(!l)
return ALLOC_FAIL;
l->head = l->tail = NULL;
l->size = 0;
return l;
}
node* getNode(int data)
{
node* newnode = malloc(sizeof(node));
if(!newnode)
return ALLOC_FAIL;
newnode->data = data;
newnode->next = NULL;
return newnode;
}
node* insertAtFront(list* l,node* newnode)
{
if(l->head)
newnode->next = l->head;
else
l->tail = newnode;
++l->size;
return l->head = newnode;
}
node* insertAtLast(list* l,node* newnode)
{
if(l->tail)
l->tail->next = newnode;
else
l->head = newnode;
++l->size;
return l->tail = newnode;
}
node* insertNode(list* l,node* newnode,node* current)
{
if(!current)
return NULL;
newnode->next = current->next;
current->next = newnode;
++l->size;
return newnode;
}
node* insertBefore(list* l,node* newnode,int searchData)
{
node* current = l->head,*previous = l->head;
if(!l->head)
return NULL;
if(l->head->data == searchData)
return insertAtFront(l,newnode);
while((current = current->next)){
if(current->data == searchData)
return insertNode(l,newnode,previous);
previous = current;
}
return NULL;
}
node* insertAfter(list* l,node* newnode,int searchData)
{
node* current = l->head;
while(current){
if(current->data == searchData)
break;
current = current->next;
}
return insertNode(l,newnode,current);
}
node* insertAtIndex(list* l,node* newnode,size_t index)
{
node* current = l->head;
if(index == 0)
return insertAtFront(l,newnode);
while(current && --index)
current = current->next;
return insertNode(l,newnode,current); // insertNode will handle if current is NULL //
}
node* nodeAt(list* l,size_t position)
{
node* current = l->head;
while(current && position--)
current = current->next;
return current;
}
void printList(list* l)
{
node* current = l->head;
printf("\n[");
while(current){
printf(" %d ,",current->data);
current = current->next; // prints [] if list is empty //
}
printf("]\n");
}
void* destroyList(list* l)
{
node* current;
while((current = l->head)){
l->head = l->head->next;
free(current);
}
return NULL; // null initialize the list variable in main //
}
main.c
#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>
#include "list.h"
static void error(char* errMsg);
static bool usrInput(char* msg,int* location);
static void clearBuffer(void);
int main(void)
{
size_t size = 5;
int data,index;
bool readSuccess;
node* newnode, *insertedNode;
list* l = createList();
if(!l){
error("List allocation failed");
return EXIT_FAILURE;
}
//inserting into list //
for(size_t i = 0; i < size; ++i){
readSuccess = usrInput("Enter your data :",&data);
if(!readSuccess)
return EXIT_FAILURE; // mem leak1 ?? Any memory leaks? os will release memory ?//
newnode = getNode(data);
if(!newnode){
error("Newnode allocation failed");
return EXIT_FAILURE; // mem leak2 ?? //
}
insertAtLast(l,newnode);
}
printf("\nYour intial list is : \n");
printList(l);
// inserting at an index //
readSuccess = usrInput("Enter position : ",&index);
if(!readSuccess)
return EXIT_FAILURE;
readSuccess = usrInput("Enter your data :",&data);
if(!readSuccess)
return EXIT_FAILURE;
newnode = getNode(data); // same code repetition! should i provide wrapper function?//
if(!newnode){
error("Newnode allocation failed");
return EXIT_FAILURE;
}
insertedNode = insertAtIndex(l,newnode,index);
if(!insertedNode){
error("No such position found in list");
free(newnode);
}
else
printf("\nYour new list is : \n");
printList(l);
l = destroyList(l); // initializing l to null (avoiding bad pointer) //
return EXIT_SUCCESS;
}
static void error(char* errMsg)
{
fprintf(stdout,"\nError : %s\n",errMsg);
}
static void clearBuffer(void){
while(getchar() != '\n');
}
static bool usrInput(char* msg,int* location)
{
bool readStatus = false;
printf("\n%s\n",msg);
if(scanf("%4d",location) == 1)
readStatus = true;
else
fprintf(stdout,"\nERROR reading input!\nPlease enter an integer value within range\n");
clearBuffer();
return readStatus;
}
//
) is, you know, unnecessary. Spend more time ensuring legibility with consistent// yadda-yadda
using a single space after the leading//
for easy reading... Pointless flourishes like this seem harmless, but are, in fact, annoying (ie: distracting) to the reader. (How many times is<stdio.h>
included in each compilation unit? (ie: source file that is compiled.) \$\endgroup\$