Merging sorted linked lists - C++

Question

Problem from hackerrank:

You’re given the pointer to the head nodes of two sorted linked lists. The data in both lists will be sorted in ascending order. Change the next pointers to obtain a single, merged linked list which also has data in ascending order. Either head pointer given may be null meaning that the corresponding list is empty.

Input Format

You have to complete the Node* MergeLists(Node* headA, Node* headB) method which takes two arguments - the heads of the two sorted linked lists to merge. You should NOT read any input from stdin/console.

Output Format

Change the next pointer of individual nodes so that nodes from both lists are merged into a single list. Then return the head of this merged list. Do NOT print anything to stdout/console.

Sample Input

1 -> 3 -> 5 -> 6 -> NULL
2 -> 4 -> 7 -> NULL

15 -> NULL
12 -> NULL

NULL 
1 -> 2 -> NULL

Sample Output

1 -> 2 -> 3 -> 4 -> 5 -> 6 -> 7
12 -> 15 -> NULL
1 -> 2 -> NULL

/*
Merge two sorted lists A and B as one linked list
Node is defined as 
struct Node
{
 int data;
 struct Node *next;
}
*/
Node* MergeLists(Node *headA, Node* headB)
{
// This is a "method-only" submission. 
// You only need to complete this method 
if(headA == NULL)
  return headB;
if(headB == NULL)
  return headA;

Node* temp1;
Node* temp2;
Node* originalHead;
Node* head = new Node;
head->data = 0;
head->next = 0;
//temp1 should always point to head with smaller value
if(headA->data <= headB->data)
{
  temp1 = headA;
  originalHead = headA;
  temp2 = headB;
}
else
{ 
  originalHead = headB;
  temp1 = headB;
  temp2 = headA;
}
while(temp1 != 0 && temp2 != 0)
{
    if(temp1->data <= temp2->data){
        head->next = temp1;
        head = temp1;
        if(temp1->next != NULL)
            temp1 = temp1->next;       
        else{
            head->next = temp2;
            break;
        }
    }
    else{
        head->next = temp2;
        head = temp2;
        if(temp2->next != NULL)
            temp2 = temp2->next;
        else{
            head->next = temp1;
            break;
        }
    }
}
return originalHead;
}

Answer 1

Simplify

In this loop, the loop condition is practically useless:

while(temp1 != 0 && temp2 != 0)
{
    if(temp1->data <= temp2->data){
        head->next = temp1;
        head = temp1;
        if(temp1->next != NULL)
            temp1 = temp1->next;       
        else{
            head->next = temp2;
            break;
        }
    }
    else{
        head->next = temp2;
        head = temp2;
        if(temp2->next != NULL)
            temp2 = temp2->next;
        else{
            head->next = temp1;
            break;
        }
    }
}

The statements before the loop have already checked that temp1 and temp2 are not null. So the condition will be true for the first time. Then in each cycle, you check if the next value of temp1 or temp2 will be null and if yes then break out. So you could as well change the loop condition to while (true), and the program will still work.

But instead of doing that, it would be simpler to move those checks out of the loop body, and let the loop condition be useful:

do {
    if (temp1->data <= temp2->data) {
        head->next = temp1;
        head = temp1;
        temp1 = temp1->next;       
    } else {
        head->next = temp2;
        head = temp2;
        temp2 = temp2->next;
    }
} while (temp1 != NULL && temp2 != NULL);

head->next = temp1 != NULL ? temp1 : temp2;

Taking it one step further, head can be updated outside of the if-else, like this:

do {
    if (temp1->data <= temp2->data) {
        head->next = temp1;
        temp1 = temp1->next;       
    } else {
        head->next = temp2;
        temp2 = temp2->next;
    }
    head = head->next;
} while (temp1 != NULL && temp2 != NULL);

head->next = temp1 != NULL ? temp1 : temp2;

You have this comment:

//temp1 should always point to head with smaller value

Nope, not really! This would work just as well:

if(headA->data <= headB->data) {
  originalHead = headA;
} else { 
  originalHead = headB;
}
temp1 = headA;
temp2 = headB;

Memory management

You created a new Node for head, but you forgot to delete it.

Suggested implementation

Some further simplifications and improvements are possible:

• No need to check if one of the heads are null. It's possible to rewrite using a dummy a node to handle such cases naturally without special treatment
• The variable names can be improved

Implementation:

Node* MergeLists(Node *headA, Node* headB)
{
    Node *dummy = new Node();
    Node *node = dummy;

    Node *nodeA = headA;
    Node *nodeB = headB;
    while (nodeA != NULL && nodeB != NULL) {
        if (nodeA->data <= nodeB->data) {
            node->next = nodeA;
            nodeA = nodeA->next;
        } else {
            node->next = nodeB;
            nodeB = nodeB->next;
        }
        node = node->next;
    }

    node->next = nodeA != NULL ? nodeA : nodeB;

    Node *head = dummy->next;
    delete dummy;
    return head;
}