# Swapping adjacent nodes of a linked list

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;
}
return curr;
}

• I might pick a slightly more specific name. Like, say, swapWithNext. – cHao May 12 '14 at 5:34

• 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;
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;