Merge Two Sorted List Leetcode Question 21

Question

So I keep getting a maximum recursion error on line 22, for this test case list1 is 1,2,4 list2 is 1,3,4 Output is 1,1,2,3,4,4 can someone explain what is causing this infinite loop to occur, python reads from top to bottom and left to right so it should have no issue with going to the first if statement and then skipping it if the if statement isn't true.

class Solution(object):
def mergeTwoLists(self, list1, list2):
    if not list1 and not list2:
        return list1
    if not list1 and list2:
        return list2
    if not list2 and list1:
        return list1
    New_Node = ListNode(0)
    temp = New_Node
    
    def Traversal(list1, list2, temp):
        if not list1:
            temp.next = list2
            return
        if not list2:
            temp.next = list1
            return

        if list1.val >= list2.val:
            temp.next = list2
            Traversal(list1, list2.next, temp.next)
        if list2.val >= list1.val:
            temp.next = list1
            Traversal(list1.next, list2, temp.next)
    
    Traversal(list1, list2, temp)
    return New_Node.next

Answer 1

The issue lies in the condition checks within the Traversal function. Currently, the code compares the values of the nodes in list1 and list2 to determine which one should be appended to the temp list. However, when the values are equal, the code doesn't handle it properly, resulting in an infinite loop.

To fix this issue, you need to modify the condition checks to include equality cases and use <= and >= instead of just < and >

class Solution(object):
    def mergeTwoLists(self, list1, list2):
        if not list1 and not list2:
            return list1
        if not list1 and list2:
            return list2
        if not list2 and list1:
            return list1
        
        new_node = ListNode(0)
        temp = new_node

        def traversal(list1, list2, temp):
            if not list1:
                temp.next = list2
                return
            if not list2:
                temp.next = list1
                return

            if list1.val <= list2.val:
                temp.next = list1
                traversal(list1.next, list2, temp.next)
            else:
                temp.next = list2
                traversal(list1, list2.next, temp.next)

        traversal(list1, list2, temp)
        return new_node.next

Answer 2

As explained in comments, you should not have a second if block that can be executed even when the first if block had already executed... That second block should only be executed when the first didn't -- so using an else.

Let's visualise what happens in the faulty code with a very small example. Take as input two lists which each just have one node with value 1.

When the solution code is run, a dummy node New_Node is created, and then Traversal is called. We can picture that state as follows:

                 list1
                   │
                ┌──┴─────────┐
                │ val: 1     │
 New_Node       │ next: None │
   │            └────────────┘
┌──┴─────────┐
│ val: 0     │
│ next: None │
└──┬─────────┘
   │            ┌────────────┐
 temp           │ val: 1     │
                │ next: None │
                └──┬─────────┘
                   │
                 list2

The first if block assigns to temp.next and makes a recursive call: I'll denote the new local variables -- that only live inside that recursive execution context!! -- with accents:

                 list1'
                 list1
                   │
                ┌──┴─────────┐
                │ val: 1     │
 New_Node       │ next: None │
   │            └────────────┘
┌──┴─────────┐
│ val: 0     │
│ next: ──────┐
└──┬─────────┘│
   │          │ ┌────────────┐
 temp         └─┤ val: 1     │
                │ next: None │
                └──┬─────────┘
                   │                
                 list2            list2' ───None
                 temp'

In the recursive execution context, list2' is None (a base case), so temp'.next = list1' is executed and the function returns to the first execution context of Traversal. Note that in that execution context we still have the original list1 and list2; recursive executions have their own set of local variables (which happen to have the same names). So coming back from the recursive call, we have this:

                 list1
                   │
                ┌──┴─────────┐
                │ val: 1     │
 New_Node       │ next: None │
   │            └─────────┬──┘
┌──┴─────────┐            │
│ val: 0     │            │
│ next: ──────┐           │
└──┬─────────┘│           │
   │          │ ┌─────────│──┐
 temp         └─┤ val: 1  │  │
                │ next: ──┘  │
                └──┬─────────┘
                   │                
                 list2

Because of the bug, the second if block executes too, and there temp.next is overwritten with temp.next = list1 (that's not how it is supposed to work), and another recursive call is made:

                 temp'
                 list1            list1' ───None 
                   │
                ┌──┴─────────┐
                │ val: 1     │
 New_Node     ┌─┤ next: None │
   │          │ └─────────┬──┘
┌──┴─────────┐│           │
│ val: 0     ││           │
│ next: ──────┘           │
└──┬─────────┘            │
   │            ┌─────────│──┐
 temp           │ val: 1  │  │
                │ next: ──┘  │
                └──┬─────────┘
                   │                
                 list2
                 list2'

Also this time the recursive call hits a base case because list1' is None. temp'.next gets assigned list2', which creates a problematic cycle, and the recursive call returns:

                 list1
                   │
                ┌──┴─────────┐
                │ val: 1     │
 New_Node     ┌─┤ next: ───┐ │
   │          │ └─────────┬│─┘
┌──┴─────────┐│           ││
│ val: 0     ││           ││
│ next: ──────┘           ││
└──┬─────────┘            ││
   │            ┌─────────│┴─┐
 temp           │ val: 1  │  │
                │ next: ──┘  │
                └──┬─────────┘
                   │                
                 list2

Things have gone really wrong: here there is no infinite recursion, because I used small lists, but it does show that a list has been created with a cycle. If one of the input lists would have had one more node, then the recursion would get stuck in this cycle (as it passes list1.next or list2.next as argument, which is a node that was already visited), finding no end to the list.

The correct algorithm requires that once temp.next has received a node reference, it should never again be overwritten. Yet this is what happens if you don't avoid the case where the two if blocks are entered and they are not mutually exclusive.