current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 5 down vote favorite

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;
}
share|improve this question
    
More linked list code is at ideone.com/UrtaO –  xyz May 30 '11 at 13:36
1  
I've always like curr for "current node pointer" to match next and previous. –  David Harkness May 30 '11 at 21:21

1 Answer 1

up vote 6 down vote accepted 
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;
}
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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