Search code examples
javaalgorithmsortingrecursionquicksort

Stackoverflow error in my quicksort algorithm


I've been working on this code for a few days with no luck because of a stack overflow error during recursion.

The problem states that our pivot is always the 0th element and through recursion we can sort an array by either swapping or creating new arrays and copy them into the original array. I chose to create 2 arrays since it is easier for me to understand but once I reach the recursive step i get this error.

I traced the algorithm and realized that the pivot element is always being placed in the lower array and may be the problem.

enter code here
public static void main(String[] args) {
    int[] arr = new int[] { 12, 7, 5, 8, 4, 6, 11, 15 };
    quicksort(arr);
    for (int i = 0; i < arr.length; i++) {
        System.out.print(arr[i] + " ");
    }
    System.out.println();

}



    private static int[] quicksort(int[] arr, int middle) {
    // Base case
    if (arr.length == 1) {
        return arr;
    }
    int[] upper = new int[arr.length];
    int[] lower = new int[arr.length];
    int upperIndex = 0, lowerIndex = 0;

    // Put the elements into their respective pivots
    for (int i = 0; i < arr.length; i++) {
        if (arr[i] <= middle) {
            lower[lowerIndex++] = arr[i];
        } else {
            upper[upperIndex++] = arr[i];
        }
    }

    // Recurse, and sort the pivots
    lower = quicksort(lower, lower[0]);
    upper = quicksort(upper, upper[0]);

    // Combine lower, middle, and upper back into one array
    System.arraycopy(lower, 0, arr, 0, lowerIndex);
    arr[lowerIndex + 1] = middle;
    System.arraycopy(upper, 0, arr, lowerIndex + 2, upperIndex);
    return arr;
}

The result should be a sorted array in ascending order.


Solution

  • As mentioned in Quimby's answer, both lower and upper are set to size arr.length. The code needs use a third parameter indicating the actual length, such as

    quicksort(lower, lower[0], lowerIndex);
    quicksort(upper, upper[0], upperIndex);
    

    Since middle will end up in lower, if the data is already sorted, than at each level of recursion, all of the data ends up in lower, with zero elements in upper, resulting in an infinite recursion that is only stopped by stack overflow.

    Since you're using separate arrays for the quicksort, you might want to consider a 3 way partition:

        public static void quicksort(int[] arr, int pivot, int size) {
            // Base case
            if (size < 2) {
                return;
            }
    
            int[] upper  = new int[size];
            int[] middle = new int[size];
            int[] lower  = new int[size];
            int upperIndex = 0, middleIndex = 0, lowerIndex = 0;
    
            // Put the elements into their respective arrays
            for (int i = 0; i < size; i++) {
                if (arr[i] < pivot) {
                    lower[lowerIndex++] = arr[i];
                } else if (arr[i] == pivot){
                    middle[middleIndex++] = arr[i];
                } else {
                    upper[upperIndex++] = arr[i];
                }
            }
    
            // sort lower and upper
            quicksort(lower,  lower[0], lowerIndex);
            quicksort(upper,  upper[0], upperIndex);
    
            // Combine lower, middle, and upper back into one array
            System.arraycopy(lower,  0, arr, 0, lowerIndex);
            System.arraycopy(middle, 0, arr, lowerIndex, middleIndex);
            System.arraycopy(upper, 0, arr, lowerIndex+middleIndex, upperIndex);
        }