2
def merge(arr,l,m,h):
    lis = []
    l1 = arr[l:m]
    l2 = arr[m+1:h]
    while((len(l1) and len(l2)) is not 0):
        if l1[0]<=l2[0]:
            x = l1.pop(0)
        else:
        x = l2.pop(0)
        lis.append(x)
    return lis

def merge_sort(arr,l,h): generating them
    if l<h:
        mid  = (l+h)//2
        merge_sort(arr,l,mid)
        merge_sort(arr,mid+1,h)
        arr = merge(arr,l,mid,h)
    return arr

arr = [9,3,7,5,6,4,8,2]
print(merge_sort(arr,0,7))

Can anyone please enlighten where my approach is going wrong ? I get only [6,4,8] as the answer. I'm trying to understand the algo and implement the logic my own way. Please help.

14
  • 2
    Debug the small failing example and and compare it with what you expected to happen (even possible with pen and paper). Commented Jan 4, 2022 at 12:33
  • 1
    Try to explain the variable names to simplify the understanding of your code, please Commented Jan 4, 2022 at 12:44
  • 1
    @JaideepSingh Just noticing that the end result with input [9,3,7,5,6,4,8,2] is [6,4,8] is not enough. Add some print statements in the code to print intermediary results. For instance, add print('sort: ', arr, l, h) at the beginning of merge_sort, and print('merge: ', arr, l, m, h) at the beginning of merge. Commented Jan 4, 2022 at 12:45
  • 1
    Oh, by the way, here is an obvious error in the code: ((len(l1) and len(l2)) is not 0) definitely does not do what you think. Instead, try (len(l1) > 0) and (len(l2) > 0) Commented Jan 4, 2022 at 12:46
  • 1
    Oops, somehow I misread, I though you were calling .pop(), not .pop(0). I'd still recommend dealing with the "end" of the list rather than the "beginning" of the list, since popping from the beginning is a very costly operation (when an element is popped, every element after it is "shifted" towards the beginning to fill the gap; if you pop the first element, the whole list is rewritten). Commented Jan 4, 2022 at 12:56

1 Answer 1

4

Several issues:

  • As you consider h to be the last index of the sublist, then realise that when slicing a list, the second index is the one following the intended range. So change this:

    Wrong Right
    l1 = arr[l:m] l1 = arr[l:m+1]
    l2 = arr[m+1:h] l2 = arr[m+1:h+1]
  • As merge returns the result for a sub list, you should not assign it to arr. arr is supposed to be the total list, so you should only replace a part of it:

    arr[l:h+1] = merge(arr,l,mid,h)
    
  • As the while loop requires that both lists are not empty, you should still consider the case where after the loop one of the lists is still not empty: its elements should be added to the merged result. So replace the return statement to this:

    return lis + l1 + l2
    
  • It is not advised to compare integers with is or is not, which you do in the while condition. In fact that condition can be simplified to this:

    while l1 and l2:
    

With these changes (and correct indentation) it will work.

Further remarks:

This implementation is not efficient. pop(0) has a O(n) time complexity. Use indexes that you update during the loop, instead of really extracting the values out the lists.

It is more pythonic to let h and m be the indices after the range that they close, instead of them being the indices of the last elements within the range they close. So if you go that way, then some of the above points will be resolved differently.

Corrected implementation

Here is your code adapted using all of the above remarks:

def merge(arr, l, m, h):
    lis = []
    i = l
    j = m
    while i < m and j < h:
        if arr[i] <= arr[j]:
            x = arr[i]
            i += 1
        else:
            x = arr[j]
            j += 1
        lis.append(x)
    return lis + arr[i:m] + arr[j:h]

def merge_sort(arr, l, h):
    if l < h - 1:
        mid  = (l + h) // 2
        merge_sort(arr, l, mid)
        merge_sort(arr, mid, h)
        arr[l:h] = merge(arr, l, mid, h)
    return arr


arr = [9, 3, 7, 5, 6, 4, 8, 2]
print(merge_sort(arr,0,len(arr)))
Sign up to request clarification or add additional context in comments.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.