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

Utility functions for a linked list

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

Problem

I have written code to add a node to the head of the list. Criticisms and feedback are welcome.

typedef struct sList
{
    void *data;
    struct sList *nextNode;
}sList;

sList *addNode2Head(sList *psList,void *data)
{
    if(psList == NULL)
        psList = newNode();
    else
    {
        psList = addHeadNode(psList);
        psList->data = data;
    }
    return psList;
}

sList *newNode(void)
{
    sList *psList =(sList *) malloc(sizeof(sList));
    //sList *psList = malloc(sizeof *sList); //Is it correct way ? if so why ?
    psList->nextNode = NULL;
    return psList;
}

sList *addHeadNode(sList *psList)
{
   sList *psListTemp = newNode();
   psListTemp->nextNode = psList;
   return psListTemp;
   //Another way ,
   //psList = psListTemp; //No return needed
}

Solution

-
The name addNode2Head will be better as addNodeToHead. Using the number 2 for the word to is rather poor choice in a function name.

-
The function addNodeToHead can be simplified to:

sList *addNodeToHead(sList *psList, void *data)
{
   sList* node = newNode();
   node->data = data;

   // It doesn't matter what psList is. The new
   // node is now the head of the list.
   node->nextNode = psList;

   return node;
}


-
Given the simplified implementation of addNodeToHead, the function addHeadNode might be unnecessary.

-
Don't explicitly cast the return value of malloc. See Do I cast the result of malloc?

Instead of

sList *psList =(sList *) malloc(sizeof(sList));


use

sList *psList = malloc(sizeof(sList));


-
Always check the return value of malloc before proceeding to use it.

sList *newNode(void)
{
    sList *psList = malloc(sizeof(sList));
    if ( psList == NULL )
    {
       return NULL;
    }
    psList->nextNode = NULL;
    return psList;
}


-
Make sure that you check the value returned by the above function.

sList* node = newNode();
   if ( node == NULL )
   {
      // Decide how you want to handle the error.
      // return NULL???
   }

   node->data = data;

Code Snippets

sList *addNodeToHead(sList *psList, void *data)
{
   sList* node = newNode();
   node->data = data;

   // It doesn't matter what psList is. The new
   // node is now the head of the list.
   node->nextNode = psList;

   return node;
}
sList *psList =(sList *) malloc(sizeof(sList));
sList *psList = malloc(sizeof(sList));
sList *newNode(void)
{
    sList *psList = malloc(sizeof(sList));
    if ( psList == NULL )
    {
       return NULL;
    }
    psList->nextNode = NULL;
    return psList;
}
sList* node = newNode();
   if ( node == NULL )
   {
      // Decide how you want to handle the error.
      // return NULL???
   }

   node->data = data;

Context

StackExchange Code Review Q#104554, answer score: 3

Revisions (0)

No revisions yet.