Reputation: 3191
The following implementation of QuickSort
runs into infinite loop
def partition(arr, lo, hi):
pivot = lo
for i in range(lo+1, hi+1):
if arr[i] <= arr[lo]:
pivot += 1
arr[i], arr[pivot] = arr[pivot], arr[i]
arr[lo], arr[pivot] = arr[pivot], arr[lo]
return pivot
def quickSort(arr, lo=0, hi=None):
if not hi: hi = len(arr) - 1
if lo >= hi: return
pivot = partition(arr, lo, hi)
quickSort(arr, lo, pivot-1)
quickSort(arr, pivot+1, hi)
arr = [5,3,2,-9,1,6,0,-1,9,6,2,5]
quickSort(arr)
print(arr)
I presume the partition
function is the culprit. Not able to figure out the mistake.
Thanks
Upvotes: 0
Views: 379
Reputation: 3927
At one point, your loop never works in partition def. See below
[5, 3, 2, -9, 1, 6, 0, -1, 9, 6, 2, 5]
lo 0 hi 11
pivot 8
[5, 3, 2, -9, 1, 0, -1, 2, 5, 6, 6, 9]
lo 0 hi 7
pivot 7
[2, 3, 2, -9, 1, 0, -1, 5, 5, 6, 6, 9]
lo 0 hi 6
pivot 5
[-1, 2, -9, 1, 0, 2, 3, 5, 5, 6, 6, 9]
lo 0 hi 4
pivot 1
[-9, -1, 2, 1, 0, 2, 3, 5, 5, 6, 6, 9]
lo 0 hi 11
pivot 0
[-9, -1, 2, 1, 0, 2, 3, 5, 5, 6, 6, 9]
lo 1 hi 11
partition doesn't seem to be doing the entire job and correctly, it is a two step process. See a sample partition def below. Also, you can refer to original source here:
def partition(alist,first,last):
pivotvalue = alist[first]
leftmark = first+1
rightmark = last
done = False
while not done:
while leftmark <= rightmark and alist[leftmark] <= pivotvalue:
leftmark = leftmark + 1
while alist[rightmark] >= pivotvalue and rightmark >= leftmark:
rightmark = rightmark -1
if rightmark < leftmark:
done = True
else:
temp = alist[leftmark]
alist[leftmark] = alist[rightmark]
alist[rightmark] = temp
temp = alist[first]
alist[first] = alist[rightmark]
alist[rightmark] = temp
return rightmark
Upvotes: 3
Reputation: 104712
The problem is with your initialization code for hi
:
if not hi: hi = len(arr) - 1
The condition not hi
is True
if hi
is zero.
This causes quicksort(arr, 0, 0)
and quicksort(arr, 1, 0)
(one of which will almost always occur in the process of sorting) to try to sort most of the list again, rather than being an end to the recursion.
You should use the more specific test if hi is None
instead of if not hi
. This way you'll never incorrect reinitialize hi
if it's zero, and the initialization code will only run if hi
is None
because it was not passed in to the function by the user.
Upvotes: 1