Swapping adjacent nodes of a linked list

Question

I just have a small question. The code I wrote works well for the problem. But is it the best way, or can we make it better?

struct node * swapAdjacent(struct node * list)
{
   struct node * temp,*curr,*nextNode;
   temp = list;
   curr = temp->next;
   if(curr == NULL)
      return temp;
   nextNode = curr->next;
   curr->next = temp;
   if(nextNode == NULL) 
   {
       temp->next = nextNode;
       return curr;
   }
   temp->next = swapAdjacent(nextNode);
   return curr;
}

Answer 1

• Consider having swapAdjacent(NULL) return NULL. Once it does, you can get rid of the whole if (nextNode == NULL) statement, and just unconditionally say temp->next = swapAdjacent(nextNode);.

• You might want to use a couple of guard clauses to separate the null checks from the other stuff. That can make the steps easier to follow.

With those things done:

struct node * swapAdjacent(struct node * list) {
   struct node *temp, *curr, *nextNode;

   if (!list) return NULL;
   if (!list->next) return list;

   temp = list;
   curr = list->next;
   nextNode = curr->next;

   curr->next = temp;
   temp->next = swapAdjacent(nextNode);
   return curr;
}

• The meanings of the names curr, temp, and nextNode are a bit foggy. I'd change the names to something that unambiguously refers to the nodes' positions in the list either before or after the swap.

• Frankly, temp and nextNode could probably go away, and the resulting code would be simpler for it.

Watch:

struct node * swapAdjacent(struct node * list) {
   if (!list) return NULL;
   if (!list->next) return list;

   struct node *newHead = list->next;
   list->next = swapAdjacent(newHead->next);
   newHead->next = list;

   return newHead;
}

Note how there's a bit of a rotation of values going on. It was going on in the original code, too...but it was harder to see with the variables in the way.