1

Here is my code for "Merge Two Sorted Lists" algorithm problem on Leetcode:

/**
 * Definition for singly-linked list.
 * struct ListNode {
 *     int val;
 *     ListNode *next;
 *     ListNode(int x) : val(x), next(NULL) {}
 * };
 */
class Solution {
public:
    ListNode* mergeTwoLists(ListNode* l1, ListNode* l2) {
        ListNode *dummy, *pre;
        dummy->next = l1;
        pre = dummy;
        while(l1 != NULL & l2 != NULL) {
            if(l1->val < l2->val) {
                pre = l1;
                l1 = l1->next;
            } else {
                pre->next = l2;
                l2->next = l1;
                pre = l2;
                l2 = l2->next;
            }
        }
        if(l2 != NULL) {
            pre->next = l2;
        }
        return dummy->next;

    }
};

And I got a Runtime Error. But what is wrong with my code?

user2916610
  • 765
  • 1
  • 5
  • 12

4 Answers4

2

I believe that a correct implementation will require substantially more code than what you had in the OP. Here is a correct implementation which you can try. I assume that the input lists l1 and l2 are sorted in descending order (i.e. largest to smallest from head to tail).

class Solution {
public:
    ListNode* mergeTwoLists(ListNode* l1, ListNode* l2) {
        ListNode *pnt1 = l1;
        ListNode *pnt2 = l2;
        ListNode *head;

        // assign the head pointer to larger head of the two input lists
        if (l1->val > l2->val) {
            head = l1;
        }
        else {
            head = l2;
        }

        // walk through both lists sequentially,
        // and splice together the sorted list
        while (pnt1->next != NULL & pnt2->next != NULL) {
            if(pnt2->val > pnt1->next->val && pnt1->val > pnt2->val) {
                ListNode* next = pnt1->next;
                pnt1->next = pnt2;
                pnt1 = next;
            }
            else if(pnt2->val > pnt1->next->val && pnt1->val <= pnt2->val) {
                ListNode* next = pnt2->next;
                pnt2->next = pnt1;
                pnt2 = next;
            }
            else if(pnt2->val <= pnt1->next->val && pnt1->val > pnt2->val) {
                pnt1 = pnt1->next;
            }
        }

        // handle edge case where end of one or two list(s) has been reached
        if (pnt1->next == NULL && pnt2->next == NULL) {
            if (pnt1->val > pnt2->val) {
                pnt1->next = pnt2;
            }
            else {
                pnt2->next = pnt1;
            }
        }
        else if (pnt1->next == NULL) {
            while (pnt2->next != NULL) {
                if (pnt1->val > pnt2->next->val) {
                    ListNode* next = pnt2->next;
                    pnt2->next = pnt1;
                    pnt1->next = next;
                    break;
                }
                pnt2 = pnt2->next;
            }
            if (pnt2->next == NULL) {
                pnt2->next = pnt1;
            }
        }
        else if (pnt2->next == NULL) {
            while (pnt1->next != NULL) {
                if (pnt2->val > pnt1->next->val) {
                    ListNode* next = pnt1->next;
                    pnt1->next = pnt2;
                    pnt2->next = next;
                    break;
                }
                pnt1 = pnt1->next;
            }
            if (pnt1->next == NULL) {
                pnt1->next = pnt2;
            }
        }

        return head;
    }
};
Tim Biegeleisen
  • 502,043
  • 27
  • 286
  • 360
  • 1
    His original code appeared to be a mess, and I thought writing a fresh answer would be more beneficial. There is no obligation on Stack Overflow to use the original code if it has problems. – Tim Biegeleisen Aug 19 '15 at 06:59
0

I think you got Segmentation Fault(Core Dump) because you are trying to access the memory that is not valid:

dummy->next = l1;

You should allocate memory to the *dummy and *pre before accessing their members.

Also use &&(logical operator) instead of &(bitwise operator) in the loop. Replace:

while(l1 != NULL & l2 != NULL) {

with

while(l1 != NULL && l2 != NULL) {

Use new operator to allocate memory and please use delete to free the same and avoid memory leaks.

Also note that the implementation itself is faulty by logic. Please refer here for better implementations.

Here is a simple recursive implementation:

ListNode* mergeTwoLists(ListNode* l1, ListNode* l2) { 
{
  ListNode* result = NULL;

  /* Base cases */
  if (l1 == NULL) 
     return (l2);
  else if (l2 == NULL) 
     return (l1);

  if (l1->data <= l2->data) 
  {
     result = l1;
     result->next = mergeTwoLists(l1->next, l2);
  }
  else
  {
     result = l2;
     result->next = mergeTwoLists(l1, l2->next);
  }
  return(result);
}
Community
  • 1
  • 1
TryinHard
  • 4,078
  • 3
  • 28
  • 54
  • 2
    True this is the immediate cause of the error, but you do not need to allocate any memory to destructively merge two sorted lists. The OP should rewrite their code – john Aug 19 '15 at 05:49
  • The Leetcode alert "RUNTIME ERROR Last executed input: [], []" – user2916610 Aug 19 '15 at 05:57
0

The main problem in your code is that you are using:

    dummy->next = l1;

when dummy has not been initialized to point to a valid object.

You are also using a bitwise & where a logical && is appropriate.

    while(l1 != NULL & l2 != NULL) {

Here's a suggested implementation.

PS It's not been tested but it looks correct to me.

ListNode* mergeTwoLists(ListNode* l1, ListNode* l2) {

   ListNode* ret = NULL;
   ListNode* pre = NULL;

   // Make sure the start of the list to be returned points to the right
   // ListNode.
   if ( l1 != NULL && l2 != NULL )
   {
      if ( l1->val < l2->val )
      {
         ret = l1;
         l1 = l1->next;
      }
      else
      {
         ret = l2;
         l2 = l2->next;
      }
   }
   else if ( l1 != NULL )
   {
      return l1;
   }
   else
   {
      return l2;
   }

   pre = ret;

   while(l1 != NULL && l2 != NULL) {

      // Figure out where pre->next must point to.
      // Advance l1 and l2 appropriately.
      if(l1->val < l2->val) {
         pre->next = l1;
         pre = l1;
         l1 = l1->next;
      } else {
         pre->next = l2;
         pre = l2;
         l2 = l2->next;
      }
   }

   // Make sure pre->next points to the remaining ListNodes.
   // They could be in l1 or l2.
   if ( l1 != NULL )
   {
      pre->next = l1;
   }

   if( l2 != NULL)
   {
      pre->next = l2;
   }

   return ret;
}
R Sahu
  • 204,454
  • 14
  • 159
  • 270
0

In addition to the problems already pointed out, the original code doesn't handle the case where the end of list 2 is reached first, in which case, the remainder of list 1 should be appended to the merged list. Using a pointer to pointer (instead of a previous pointer) makes the code simpler. Here is example code to merge two lists and also a bottom up merge sort that uses the merge lists function. The sort uses an array of pointers to lists, where array[i] is either null or it points to a list with pow(2,i) elements in it.

ListNode * MergeLists(ListNode *pl1, ListNode *pl2)
{
ListNode *plm = NULL;                   /* merged list head ptr */
ListNode **pplm = &plm;                 /* ptr to head or prev->next */
    if(pl1 == NULL)
        return pl2;
    if(pl2 == NULL)
        return pl1;
    while(1){
        if(pl2->val < pl1->val){        /* if src2 < src1 */
            *pplm = pl2;
            pl2 = *(pplm = &(pl2->next));
            if(pl2 == NULL){
                *pplm = pl1;
                break;
            }
        } else {                        /* src1 <= src2 */
            *pplm = pl1;
            pl1 = *(pplm = &(pl1->next));
            if(pl1 == NULL){
                *pplm = pl2;
                break;
            }
        }
    }
    return plm;
}

#define NUMLISTS 32                     /* number of lists */
ListNode * SortList(ListNode *pList)
{
ListNode * aList[NUMLISTS];             /* array of lists */
ListNode * pNode;
ListNode * pNext;
int i;
    if(pList == NULL)                   /* check for empty list */
        return NULL;
    for(i = 0; i < NUMLISTS; i++)       /* zero array */
        aList[i] = NULL;
    pNode = pList;                      /* merge nodes into aList[] */
    while(pNode != NULL){
        pNext = pNode->next;
        pNode->next = NULL;
        for(i = 0; (i < NUMLISTS) && (aList[i] != NULL); i++){
            pNode = MergeLists(aList[i], pNode);
            aList[i] = NULL;
        }
        if(i == NUMLISTS)
            i--;
        aList[i] = pNode;
        pNode = pNext;
    }
    pNode = NULL;                       /* merge array into one list */
    for(i = 0; i < NUMLISTS; i++)
        pNode = MergeLists(aList[i], pNode);
    return pNode;
}
rcgldr
  • 27,407
  • 3
  • 36
  • 61