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

Inserting into a linked list

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

Problem

I know that this is very very basic, but now I am starting from group up after getting frustrated with my coding practices and knowledge of standard idioms out there and elegant way of coding corner cases. The problem is to insert into tail of a linked list.

void insertattail (struct node *head, int data)
{
  //First construct the node in newp
  struct node *newp;
  struct node *p;
  newp = malloc (sizeof (struct node));
  newp -> data = data;
  newp -> next = NULL; 

  // Standard name for node pointers only used for traversal? p? q? tmp?
  // if more than 1 kind of nodes?
  // Then what about temp pointers for swapping etc but not exactly for traversal?

  if (head == NULL)  // is there more elegant way of dealing with this? any idiom?
  {
    head = newp;
    return;
  }

  for (p=head; p->next != NULL; p++)
    ;
  p->next=newp;
}

Solution

if(head == NULL)  // is there more elegant way of dealing with this? any idiom?
{
head=newp;
return;
}


head is a local variable in this function. Modifying it will not change the variable that was passed into the function. As a result, the new node you create isn't attached to anything that the caller can access. If you want to modify a variable being passed in you need to use a pointer. If you want to modify a pointer, you need to use a pointer to a pointer.

Here is a version which reduces the amount of code, but for those who don't think in pointers naturally it may be hard to follow.

// I take a pointer to a pointer so that I can modify it if need be.
void insertattail(struct node **head,int data)
{
    // advance the pointer until it reaches NULL
    // because its a pointer to a pointer I can modify it
    while(*head != NULL) head = &(*head)->next;

    // replace that NULL with a new node
    *head = malloc(sizeof(struct node));
    // fill up the node
    (*head)->data = data;
    (*head)->next = NULL;
}

Code Snippets

if(head == NULL)  // is there more elegant way of dealing with this? any idiom?
{
head=newp;
return;
}
// I take a pointer to a pointer so that I can modify it if need be.
void insertattail(struct node **head,int data)
{
    // advance the pointer until it reaches NULL
    // because its a pointer to a pointer I can modify it
    while(*head != NULL) head = &(*head)->next;

    // replace that NULL with a new node
    *head = malloc(sizeof(struct node));
    // fill up the node
    (*head)->data = data;
    (*head)->next = NULL;
}

Context

StackExchange Code Review Q#2710, answer score: 7

Revisions (0)

No revisions yet.