Merging sorted linked lists - C++

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 data)
{
temp1 = headA;
originalHead = headA;
temp2 = headB;
}
else
{
originalHead = headB;
temp1 = headB;
temp2 = headA;
}
while(temp1 != 0 && temp2 != 0)
{
if(temp1->data 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{

Simplify

In this loop, the loop condition is practically useless:

while(temp1 != 0 && temp2 != 0)
{
    if(temp1->data 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 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 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 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 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;
}

StackExchange Code Review Q#123346, answer score: 2