Inplace Quicksort in Java

Question

For refreshing some Java I tried to implement a quicksort (inplace) algorithm that can sort integer arrays. Following is the code I've got so far. You can call it by sort(a,0,a.length-1).

This code obviously fails (gets into an infinite loop) if both 'pointers' i,j point each to an array entry that have the same values as the pivot. The pivot element v is always the right most of the current partition (the one with the greatest index).

But I just cannot figure out how to avoid that, does anyone see a solution?

static void sort(int a[], int left, int right)   {
    if (right > left){
        int i=left, j=right-1, tmp;
        int v = a[right]; //pivot
        int counter = 0;
        do {
            while(a[i]<v)i++;
            while(j>0 && a[j]>v)j--;

            if( i < j){
                tmp = a[i];
                a[i] = a[j];
                a[j] = tmp;
            }
        } while(i < j);
        tmp = a[right];
        a[right] = a[i];
        a[i] = tmp;
        sort(a,left,i-1);
        sort(a,i+1,right);

    }
}    

Answer 1

When preforming a Quicksort I strongly suggest making a separate method for partitioning to make the code easier to follow (I'll show an example below). On top of this a good way of avoiding worst case run time is shuffling the array you're sorting prior to preforming the quick sort. Also I used the first index as the partitioning item instead of the last.

For example:

public static void sort (int[] a)
{
    StdRandom.shuffle(a);
    sort(a, 0, a.length - 1);
}

private static void sort(int[] a, int lo, int hi)
{
    if (hi <= lo) return;
    int j = partition(a, lo, hi) // the addition of a partitioning method
    sort(a, lo, j-1);
    sort(a, j+1, hi);
}

private static int partition(int[] a, int lo, int hi)
{
    int i = lo, j = hi + 1, tmp = 0;
    int v = a[lo];
    while (true)
    {
         while (a[i++] < v) if (i == hi) break;
         while (v < a[j--]) if (j == lo) break;
         if (i >= j) break;
         tmp = a[i];
         a[i] = a[j];
         a[j] = tmp;
    }
    tmp = a[lo];
    a[lo] = a[j];
    a[j] = temp;
    return j;
}

On top of this if you want a really good example on how Quicksort works (as a refresher) see here.

Answer 2

This should work (will check for correctness in a bit, it works!):

EDIT: I previously made a mistake in error checking. I forgot to add 2 more conditions, here is the amended code.

public static void main (String[] args) throws java.lang.Exception
{
    int b[] = {10, 9, 8, 7, 7, 7, 7, 3, 2, 1};
    sort(b,0,b.length-1);
    System.out.println(Arrays.toString(b));
}

static void sort(int a[], int left, int right)   {  
   if (right > left){
    int i=left, j=right, tmp;    
    //we want j to be right, not right-1 since that leaves out a number during recursion

    int v = a[right]; //pivot

    do {
        while(a[i]<v)
          i++;
        while(a[j]>v) 
        //no need to check for 0, the right condition for recursion is the 2 if statements below.
          j--;

        if( i <= j){            //your code was i<j
           tmp = a[i];
           a[i] = a[j];
           a[j] = tmp;
           i++;            
           j--;
           //we need to +/- both i,j, else it will stick at 0 or be same number
        }
   } while(i <= j);           //your code was i<j, hence infinite loop on 0 case

    //you had a swap here, I don't think it's needed.
    //this is the 2 conditions we need to avoid infinite loops
    // check if left < j, if it isn't, it's already sorted. Done

    if(left < j)  sort(a,left,j);
    //check if i is less than right, if it isn't it's already sorted. Done
    // here i is now the 'middle index', the slice for divide and conquer.

    if(i < right) sort(a,i,right);
  }

}

This Code in the IDEOne online compiler

Basically we make sure that we also swap the value if the value of i/j is the same as the pivot, and break out of the recursion.

Also there was a check in the pseudocode for the length, as if we have an array of just 1 item it's already sorted (we forgot the base case), I thought we needed that but since you pass in the indexes and the entire array, not the subarray, we just increment i and j so the algorithm won't stick at 0 (they're done sorting) but still keep sorting an array of 1. :)

Also, we had to add 2 conditions to check if the array is already sorted for the recursive calls. without it, we'll end up sorting an already sorted array forever, hence another infinite loop. see how I added checks for if left less than j and if i less than right. Also, at that point of passing in i and j, i is effectively the middle index we split for divide and conquer, and j would be the value right before the middle value.

The pseudocode for it is taken from RosettaCode:

function quicksort(array)
    if length(array) > 1
        pivot := select any element of array
        left := first index of array
        right := last index of array
        while left ≤ right
            while array[left] < pivot
                left := left + 1
            while array[right] > pivot
                right := right - 1
            if left ≤ right
                swap array[left] with array[right]
                left := left + 1
                right := right - 1
        quicksort(array from first index to right)
        quicksort(array from left to last index)

Reference: This SO question

Also read this for a quick refresher, it's implemented differently with an oridnary while loop

This was fun :)

Answer 3

Heres some simple code I wrote that doesn't initialize to many pointers and gets the job done in a simple manner.

public int[] quickSort(int[] x ){
    quickSortWorker(x,0,x.length-1);
    return x;
}


private int[] quickSortWorker(int[] x, int lb, int ub){
    if (lb>=ub) return x; 
    int pivotIndex = lb;
    for (int i = lb+1 ; i<=ub; i++){
        if (x[i]<=x[pivotIndex]){
            swap(x,pivotIndex,i);
            swap(x,i,pivotIndex+1);
            pivotIndex++;
        }
    }
    quickSortWorker(x,lb,pivotIndex-1);
    quickSortWorker(x,pivotIndex+1,ub);
    return x;
}

private void swap(int[] x,int a, int b){
    int tmp = x[a];
    x[a]=x[b];
    x[b]=tmp;
}