23

This is one of the programming questions asked during written test from Microsoft. I am giving the question and the answer that I came up with. Thing is my answer although looks comprehensive (at least to me), I feel that the number of lines can be reduced. It was asked in C and I am a Java person but I managed to code it (my answer may contain too many Java like syntaxes)

Ok, here is the question.

You have two lists that are already sorted, you have to merge them and return a new list without any new extra nodes. The returned list should be sorted as well.

The method signature is,

Node* MergeLists(Node* list1, Node* list2);

struct Node{
    int data;
    Node *next;
}

The following is the solution I came up with,

Node* MergeLists(Node* list1, Node* list2){
    Node* mergedList;
    if(list1 == null && list2 ==null){//if both are null, return null
        return null;
    }
    if(list1 == null){//if list1 is null, simply return list2
        return list2;
    }
    if(list2 == null){//if list2 is null, simply return list1
        return list1;
    }
    if(list1.data < list2.data){//initialize mergedList pointer to list1 if list1's data is lesser
        mergedList = list1;
    }else{//initialize mergedList pointer to list2 if list2's data is lesser or equal
        mergedList = list2;
    }
    while(list1!=null && list2!=null){
        if(list1.data < list2.data){
            mergedList->next = list1;
            list1 = list1->next;
        }else{
            mergedList->next = list2;
            list2 = list2->next;
        }
    }
    if(list1 == null){//remaining nodes of list2 appended to mergedList when list1 has reached its end.
        mergedList->next = list2;
    }else{//remaining nodes of list1 appended to mergedList when list2 has reached its end
        mergedList->next = list1;
    }
    return mergedList;
}

I am very confident this can be enhanced. Please help me find what lines are redundantly I've added. Please feel free to criticize my syntax errors and logic.

Thanks!

bragboy
  • 32,353
  • 29
  • 101
  • 167
  • 1
    Some of your code can be shortened simply by using trinary operators in the start. I.E rewrite the parameter tests using to mergedList = (list1 == null ? list2 : null) and save lines of code, though 'not' operations. – Daniel Goldberg Feb 27 '10 at 18:03
  • 1
    Bragboy, do you mind changing your title into a more descriptive form like *"Merging two sorted lists"*? Your current title (Need your advise/tips on this coding problem) is what we are all here for. :) It does not identify or promote your question. – edgerunner Oct 04 '10 at 18:04

15 Answers15

19

The most glaring bug is in your loop, you keep overwriting mergedList->next, losing the previously added node. That is, your returned list will never contain more than two nodes, regardless of input ...

It's been a while since I did C, but I would do it as follows:

Node* merge(Node* list1, Node* list2) {
    Node* merged = null;
    Node** tail = &merged;

    while (list1 && list2) {
        if (list1->data < list2->data) {
            *tail = list1;
            list1 = list1->next;
        } else {
            *tail = list2;
            list2 = list2->next;
        }
        tail = &((*tail)->next);
    }
    *tail = list1 ? list1 : list2;
    return merged;
}
meriton
  • 61,876
  • 13
  • 96
  • 163
13

Your code is overloaded with if-s inserted for handling "special" cases, which bloats it a lot and makes it difficult to read. This usually happens when you decide to handle special cases "by code" instead of finding a way to handle them "by data". A statement attributed to David Wheeler says "All problems in computer science can be solved by another level of indirection". That "extra level of indirection" usually works very well with lists, helping to significantly reduce clutter created by those ifs.

To illustrate the above, here's what my code would look like

#define SWAP_PTRS(a, b) do { void *t = (a); (a) = (b); (b) = t; } while (0)

Node* MergeLists(Node* list1, Node* list2) 
{
  Node *list = NULL, **pnext = &list;

  if (list2 == NULL)
    return list1;

  while (list1 != NULL)
  {
    if (list1->data > list2->data)
      SWAP_PTRS(list1, list2);

    *pnext = list1;
    pnext = &list1->next;
    list1 = *pnext;
  }

  *pnext = list2;
  return list;
}

Some might argue that the use of an extra level of indirection in pnext pointer actually makes the code more difficult to read. I'd agree that for a newbie the approach might pose some difficulties, but for an experienced programmer this should be easily digestible as an idiom.

AnT
  • 291,388
  • 39
  • 487
  • 734
  • Thanks. Will consider this kind of approach in attacking problems of this nature in future. That really helped! – bragboy Feb 27 '10 at 18:28
  • 3
    Wouldn't this attempt to dereference `null` when `null` is passed for `list2`? – meriton Feb 27 '10 at 18:29
  • @meriton: Yes, it would. I added an extra check :) Thank you. – AnT Feb 27 '10 at 18:30
  • 1
    +1 for explanation about indirection and simplifying the while condition to only check the modified pointer. – meriton Feb 27 '10 at 18:35
  • @polygenelubricants: A typo. The intent was to return `list1` (not `list`) when `list2` is null. Fixed. – AnT Feb 27 '10 at 18:39
  • why not change these 2 lines :pnext = &list1->next; list1 = *pnext; to list1= list1->next; ? – Josh Morrison Mar 21 '11 at 09:39
  • @Josh Morrison: Er... Because that simply wouldn't work. It is important to maintain the correct value of `pnext` at all times. The previous line `*pnext = list1` relies on it. – AnT Mar 21 '11 at 18:22
10

My take, with a test case

So far all of the answers have been interesting and well done. It's possible that this one is more like what an interviewer would like to see, featuring DRY/DIE, and TDD. :-)

#include <stdio.h>

typedef struct ns {
    int data;
    struct ns *next;
} Node;

Node l1[] = {
  { 1, &l1[1] },
  { 3, &l1[2] },
  { 5, &l1[3] },
  { 7, &l1[4] },
  { 9, &l1[5] },
  {11, 0 },
};

Node l2[] = {
  { 2, &l2[1] },
  { 4, &l2[2] },
  { 6, 0 },
};

Node* MergeLists(Node* list1, Node* list2) {
  Node *t, **xt;
  for(xt = &t; list1 || list2;) {
    Node **z = list1 == NULL ? &list2 :
               list2 == NULL ? &list1 :
               list1->data < list2->data ? &list1 : &list2;
    *xt = *z;
     xt = &(*z)->next;
    *z  = *xt;
  }
  *xt = NULL;
  return t;
}

int main(void) {
  for(Node *t = MergeLists(l1, l2); t; t = t->next) 
    printf("%d\n", t->data);
  return 0;
}
DigitalRoss
  • 135,013
  • 23
  • 230
  • 316
5

It doesn't get any more elegant than this:

Node* merge2(Node* n1, Node* n2) {
    n1->next = merge(n1->next, n2);
    return n1;
}

Node* merge(Node* n1, Node* n2) {
    return (n1 == null) ? n2 :
           (n2 == null) ? n1 :
           (n1->data < n2->data) ?
               merge2(n1, n2) :
               merge2(n2, n1);
}

Assuming that you understand recursion, this is as clear as it gets.


I should point out that this is good for an interview answer only (where presumably demonstrating clarity of thought has more impact than simply showing that you know how to write programs). In practice, you wouldn't want to merge this way, since it uses O(n) stack depth, which likely would cause a stack overflow. Also, it's not a tail-recursion, so it's not compiler-optimizable.

polygenelubricants
  • 348,637
  • 121
  • 546
  • 611
  • Well, it's concise, but I wouldn't call nested ternaries "clear." – bobbymcr Feb 27 '10 at 18:28
  • 1
    Well, it takes getting used to, but unless you abuse it, nested ternaries actually makes for a very clear code to me at least. It requires good logic and good formatting, some parentheses and some indentation, but that's about it. – polygenelubricants Feb 27 '10 at 18:31
  • i find it really hard to write a recursive method without the help of a IDE. I always want to do a test or two before actually finalizing on one. Thats the reason I try to avoid it as much as possible – bragboy Feb 27 '10 at 18:32
  • 3
    It is "elegant" in a sense that it uses recursion. But from the functional point of view, using recursion in a straightforwardly cyclical situation is not exactly an example of functional elegance. Any cycle can be replaced with recursion. But should it be? – AnT Feb 27 '10 at 18:36
  • It does make for nice code and as long as the final operation is the return, compilers can make it just as efficient as a loop. I'd say I like it. – Zan Lynx Feb 27 '10 at 18:45
  • I admit that this is not "practical", since the O(n) recursion depth would cause stack overflow, etc. As an interview answer, though, this shows that you really know your computation theory. – polygenelubricants Feb 27 '10 at 18:49
  • 2
    If I were you, I would resist the need to create an extra function :) You could've easily done it with one function by using `,` operator. The last branch would simply look as `n1->data < n2->data ? n1->next = merge(n1->next, n2), n1 : n2->next = merge(n2->next, n1), n2`. But that starts to smell of Obfuscated C Code Contest :) – AnT Feb 27 '10 at 18:56
  • 1
    Yes, the contrast between your solution and mine is that yours (after all the bugs are fixed) shows that the author knows how to write correct code. Mine shows that the author knows how to think clearly. – polygenelubricants Feb 27 '10 at 19:02
4

Divide et Impera

(i.e. MergeSort)

bragboy
  • 32,353
  • 29
  • 101
  • 167
Luca
  • 10,886
  • 10
  • 64
  • 120
  • 1
    This is exactly the correct answer, why the downvotes? @Luca: +1 from me. – tzaman Feb 27 '10 at 18:19
  • 1
    @tzaman: How on world is this a related answer to this question? – bragboy Feb 27 '10 at 18:33
  • @Bragaadeesh: Do you have to sort? I wonder why not use well known algorithms. Surely you get best performances using Divide & Impera pattern, instead of simply iterating over the two lists. It is just a critics about the algorithm pattern, indeed it seems to me pertinent. – Luca Feb 27 '10 at 18:45
  • I voted you up as well. This is exactly the merge part of mergesort. – Larry Feb 27 '10 at 19:00
  • Aw!! Come on guys, please read the question fully. This is not about sorting algorithm. The data is already sorted. This question is more about programming style. – bragboy Feb 27 '10 at 19:06
  • 1
    It's about merging two sorted lists. The second part of merge sort is merging two sorted lists. It's pretty straight forward. – Ross Feb 27 '10 at 19:29
2

Use recursion. The code is as follows:

ListNode* mergeTwoSortedLists(ListNode* pHead1, ListNode* pHead2)
{
    if(pHead1 == NULL)
        return pHead2;
    else if(pHead2 == NULL)
        return pHead1;

    ListNode* pMergedHead = NULL;

    if(pHead1->m_nValue < pHead2->m_nValue)
    {
        pMergedHead = pHead1;
        pMergedHead->m_pNext = mergeTwoSortedLists(pHead1->m_pNext, pHead2);
    }
    else
    {
        pMergedHead = pHead2;
        pMergedHead->m_pNext = mergeTwoSortedLists(pHead1, pHead2->m_pNext);
    }

    return pMergedHead;
}
herohuyongtao
  • 45,575
  • 23
  • 118
  • 159
2

So merging polygen wit AndreyT we get:

Node* merge(Node* n1, Node* n2) {
    return (n1 == null) ? n2 :
           (n2 == null) ? n1 :
             (n1->data < n2->data) ? 
               (n1->next = merge(n1->next, n2), n1) : 
               (n2->next = merge(n2->next, n1), n2)}

I can't claim credit for this one, but it is the most concise and shows the symmetry between the two arguments, doesn't introduce any obscure helper functions. I am not sure an optimizing compiler will see a tail recursion here but I do. The indentation is a final touch.

piccolbo
  • 1,267
  • 6
  • 17
0

This is my take. Unlike other solutions it identifies and skips over consecutive nodes on one list which are smaller or equal to the head node of the other list. The head of the other list is attached at the end of such sequence and the process is repeated after switching roles. This approach minimizes the number of assignments to Node.next while limiting NULL testing to single check in each iteration.

Node * merge(Node *list1, Node *list2)
{
    if (!list1) return list2;
    if (!list2) return list1;

    Node *tmp;

    // compare head nodes and swap lists to guarantee list1 has the smallest node
    if (list1->val > list2->val) {
        tmp = list2;
        list2 = list1;
        list1 = tmp;
    }

    Node *tail = list1;

    do {
        // Advance the tail pointer skipping over all the elements in the result
        // which have smaller or equal value than the first node on list2
        while (tail->next && (tail->next->val <= list2->val)) {
            tail = tail->next;
        }
        // concat list2 at tail of result and make the rest after tail list2 
        tmp = tail->next;
        tail->next = list2;
        tail = list2;
        list2 = tmp;
    } while (list2);

    return list1;
}
Ronen
  • 1
0
#include<stdio.h>

typedef struct NODE
{
    int data;
    struct NODE * next;
}NODE;

NODE * func(NODE*,NODE*);
int main()
{
    int i;
    int size;
    int value;
    NODE * head1,*head2,*newNode,*ptr,*final;
    printf("\nPlease enter the number of elements\n");
    scanf("%d",&size);

    for(i=0;i<size;i++)
    {
            printf("List 1\n");
            printf("Please enter the value number %d \n",i+1);
            scanf("%d",&value);
            newNode=(NODE*)malloc(sizeof(NODE));
            newNode->data=value;
            newNode->next=NULL;
            if(i!=0)
            {
                ptr->next=newNode;  
                ptr=ptr->next;
            }

            if(i==0)
            {
                head1=newNode;
                ptr=newNode;

            }
    }
    for(i=0;i<size;i++)
    {
            printf("\n\nList 2\n");
            printf("Please enter the value number %d \n",i+1);
            scanf("%d",&value);
            newNode=(NODE*)malloc(sizeof(NODE));
            newNode->data=value;
            newNode->next=NULL;
            if(i!=0)
            {
                ptr->next=newNode;  
                ptr=ptr->next;
            }

            if(i==0)
            {
                head2=newNode;
                ptr=newNode;

            }
    }

    final=func(head1,head2);
    printf("\n\n");
    while (final!=NULL)
    {
        printf("%d -->",final->data);
        final=final->next;
    }
    printf("NULL
    ");
    return 0;
}

NODE* func(NODE* list1, NODE* list2)
{

    NODE* mergedList,*mergeHead=NULL;
    if(list1 == NULL && list2 ==NULL){//if both are NULL, return NULL
        return NULL;
    }
    if(list1 == NULL){//if list1 is NULL, simply return list2
        return list2;
    }
    if(list2 == NULL){//if list2 is NULL, simply return list1
        return list1;
    }
    mergedList = (NODE*)malloc(sizeof(NODE));
    if(list1->data < list2->data){//initialize mergedList pointer to list1 if list1's data is lesser

        mergedList->data=list1->data;
        mergedList->next=NULL;
        list1 = list1->next;

    }else{//initialize mergedList pointer to list2 if list2's data is lesser or equal
        mergedList->data=list2->data;
        mergedList->next=NULL;
        list2 = list2->next;

    }
    mergeHead=mergedList;

    while(list1!=NULL && list2!=NULL){
        if(list1->data < list2->data){
            mergedList->next = (NODE*)malloc(sizeof(NODE));
            mergedList=mergedList->next;
            mergedList->data=list1->data;
            mergedList->next=NULL;
            list1 = list1->next;
        }else{
            mergedList->next = (NODE*)malloc(sizeof(NODE));
            mergedList=mergedList->next;
            mergedList->data=list2->data;
            mergedList->next=NULL;
            list2 = list2->next;
        }
    }
    if(list1 == NULL){//remaining nodes of list2 appended to mergedList when list1 has reached its end.
       while(list2!=NULL)
       {
            mergedList->next = (NODE*)malloc(sizeof(NODE));
            mergedList=mergedList->next;
            mergedList->data=list2->data;
            mergedList->next=NULL;
            list2 = list2->next;
       }

    }else{//remaining nodes of list1 appended to mergedList when list2 has reached its end
            mergedList->next = (NODE*)malloc(sizeof(NODE));
            mergedList=mergedList->next;
            mergedList->data=list1->data;
            mergedList->next=NULL;
            list1 = list1->next;
    }
    return mergeHead;
}
0

I have created recursion function for it. Here is my solution:

Node* merge_recursion(Node* l1, Node* l2)
{
        if (!l1)
                return l2;
        if (!l2)
                return l1;

        if (l1->data < l2->data) {
                l1->next = merge_recursion(l1->next, l2);
                return l1;
        } else {
                l2->next = merge_recursion(l1, l2->next);
                return l2;
        }
}

Store return pointer into new pointer variable (in main() / calling function) and traverse linked list on new pointer to print data, it will result sorted merged linked list.

Brijesh Valera
  • 985
  • 1
  • 7
  • 28
0

You can use recursion:

Node* MergeLists(Node *headA, Node* headB)
{

if(headA==NULL){
    return headB;
}else if(headB ==NULL){
    return headA;
}
Node* head = NULL;
if(headA->data <= headB->data){
    head= headA;
    head->next = MergeLists(headA->next,headB);
}else{
    head= headB;
    head->next = MergeLists(headA,headB->next);
}
 return head;
}
0

You may use Java 8, Stream API to merge, get Distinct and sort. Below is sample code for sorting and merging two list with Integer elements

List<Integer> list1= Arrays.asList(2,3,5,8);
List<Integer> list2 = Arrays.asList(3,6,8);

List<Integer> finalList = new ArrayList<>();
finalList.addAll(list1);
finalList.addAll(list2);

List<Integer>  mergeSortedList = 
  finalList.stream()
    .distinct()
    .sorted()
    .collect(Collectors.toList());
System.out.println(mergeSortedList);
zx485
  • 24,099
  • 26
  • 45
  • 52
0

Iterative solution for merging two sorted Linked List:

class ListNode:
    def __init__(self, val=0, next=None):

         self.val = val

         self.next = next


    class Solution:
            def mergeTwoLists(self, l1: ListNode, l2: ListNode) -> ListNode:
                head = ListNode(-200)
                prev = head
                while l1 and l2:
                    if l1.val <=l2.val:
                        prev.next = l1
                        l1 = l1.next
                    else:
                        prev.next = l2
                        l2 = l2.next
                    prev = prev.next
                    
                prev.next = l1 if l1 is not None else l2
                
                return head.next
                
Archit Pandey
  • 149
  • 1
  • 7
0
public void Merge(LinkList list1, LinkList list2) {
        if (list1.head == null && list2.head == null) {
            System.out.println("Empty list"); //Check if lists are empty
        }
        if (list1.head == null) { //If list1 is empty print list2
            list2.printList();
        }
        if (list2.head == null) { //If list2 is empty print list1
            list1.printList(); 
        }
        LinkList list3 = new LinkList(); //create a new LinkList object for merging
        Node a = list1.head; //Beginning of list1
        Node b = list2.head; //Beginning of list2
        while (a != null && b != null) { //Until list ends
            if (a.value <= b.value) { //Compare values of list1 against list2
                list3.insert(a.value); //Insert values to new list
                a = a.next;
            } else if (a.value >= b.value) {
                list3.insert(b.value);
                b = b.next;
            }  else if (a.value == b.value){ //Insert only unique values and discard repeated values
            list3.insert(a.value);
            a = a.next;
            b = b.next;
        }
        }
        if (a == null) {
            while (b != null) {
                list3.insert(b.value); //If list1 becomes empty, attach rest of the list2 to list3
                b = b.next;
            }
        }
        if (b == null) {
            while (a != null) {
                list3.insert(a.value);
                a = a.next;
            }
        }
        list3.printList(); //print merged list
    }
}

Hi guys ! I was preparing for an interview this month and while I was working on this problem, this is the solution I came up with. I compared my solution with many solutions you posted here and find my program extremely lengthy. Although I find this easier to understand and implement, is there a better solution in Java to the existing code. I'm looking for a better time complexity solution. Any help/direction/tip is appreciated.

PS: I was unable to come up with a Java solution for the programs listed above in C which used pointers.

Naveen
  • 667
  • 3
  • 11
  • 24
-1
//I have used recursions .
//Sorry for such a long code.
//:) it works,hope it helps.
#include<stdio.h>
#include<malloc.h>
#include<stdlib.h>
struct node{
    int data;
    struct node *next ;
};
struct node *start1=NULL,*start2=NULL;
struct node*start=NULL;
struct node *create_ll1(struct node *start);
struct node *create_ll2(struct node *start);
void sorted_ll(struct node* node1,struct node* node2);

struct node *display(struct node *start);
void p(struct node*);
main(){
    start1=create_ll1(start1);
    start2=create_ll2(start2);
    //start1=display(start1);
    printf("\n");
    //start2=display(start2);
    sorted_ll(start1,start2);
    //start=display(start);


}
struct node *create_ll1(struct node *start1){
    struct node *ptr,*new_node;
    int num;
    printf("Enter -1 for ending \n");
    printf("Enter data for list 1: \n");
    scanf("%d",&num);
    while(num!=-1){
        new_node=(struct node *)malloc(sizeof(struct node));
        new_node->data=num;
        if(start1==NULL){
            new_node->next=NULL ;
            start1=new_node;
        }
        else{
            ptr=start1 ;
            while(ptr->next!=NULL)
            ptr=ptr->next;
            ptr->next=new_node;
            new_node->next=NULL ;
        }
        printf("Enter data: \n");
        scanf("%d",&num);
    }
    return start1;

}
struct node *create_ll2(struct node *start2){
    struct node *ptr,*new_node;
    int num;
    printf("Enter -1 for ending \n");
    printf("Enter data for list 2: \n");
    scanf("%d",&num);
    while(num!=-1){
        new_node=(struct node *)malloc(sizeof(struct node));
        new_node->data=num;
        if(start2==NULL){
            new_node->next=NULL ;
            start2=new_node;
        }
        else{
            ptr=start2 ;
            while(ptr->next!=NULL)
            ptr=ptr->next;
            ptr->next=new_node;
            new_node->next=NULL ;
        }
        printf("Enter data: \n");
        scanf("%d",&num);
    }
    return start2;

}

struct node *display(struct node *start){
    struct node *ptr;
    ptr=start;
    while(ptr->next!=NULL){
        printf("\t %d",ptr->data);
        ptr=ptr->next;
    }
        printf("\t %d",ptr->data);
        printf("\n");


    return start ;
}
void sorted_ll(struct node* node1,struct node* node2)
{
    if(!node1){
        p(node2);
        exit(0);
    }
    else if(!node2){
        p(node1);
        exit(0);
    }
    if(node1->data<node2->data){
        printf("%d\t",node1->data);
        sorted_ll(node1->next,node2);


    }
    else{
        printf("%d\t",node2->data);
        sorted_ll(node1,node2->next);
    }
}
void p(struct node* pme){
    while(pme->next!=NULL){
        printf("%d \t",pme->data);
        pme=pme->next;
    }
    printf("%d",pme->data);

}
  • While (sort of) including a comment what all of the code is about is a good start, that comment won't work out of the context of this question. The remainder of your code is uncommented. Once you have a test frame and are confident that your codes works as specified, consider posting on [Code Review](https://codereview.stackexchange.com/help/on-topic) to live up to your user name. – greybeard Jan 01 '18 at 11:55