1

Attempting to implement QuickSort using Hoare Partition Scheme, but I am running into a problem where changing the index of the pivot causes overflow, regardless of array size. Code:

public void quickSort(int[] l, int min, int max){
    if (min < max){
        int p = partition(l, min, max);
        quickSort(l, min, p);
        quickSort(l, p+1, max);
    }
}
public int partition(int[] l, int min, int max){
    int pivot = l[min];
    int i = min - 1;
    int j = max +1;
    while(true){
        do{
            i++;
        }while(l[i] < pivot);
        do{
            j--;
        }while(l[j] > pivot);

        if (i >= j) {
                return j;
        }
        //Swap
        int temp = l[i];
        l[i] = l[j];
        l[j] = temp;
    }

}

This implementation chooses the low-index (named min here) as the pivot element, and this works just fine. However, changing the pivot element to any other index, causes a StackOverflow Error regardless of the size of the array that is being sorted. (Error refers to line 3, where partition() is called) I would preferably have the pivot element chosen at random within the (min,max) range. What is causing this?

EDIT: The array used is generated as follows:

public static int[] generateRandomArray(int size, int lower, int upper){
    int[] random = new int[size];
    for (int i = 0; i < random.length; i++) {
        int randInt = ThreadLocalRandom.current().nextInt(lower, upper+1);
        random[i] = randInt;
    }
    return random;
}

In one of the Overflow cases I used this:

genereateRandomArray(10, 0, 9);

For some concrete examples, running the code above but changing the pivot element to say, l[max-1] or l[min+1], l[min+2] etc gives StackOverflow on my end.

The solution to my problem was as user MBo pointed out to swap the pivot element to the first index of the array, as the algorithm itself relies on the pivot being on index 0. This is what I had overlooked. (int i = min - 1; is correct however, and stays that way.)

5
  • Stack overflow should only occur for pivot = l[max]. Any other index should be ok. Normally the middle element is chosen: pivot = [(min+max)/2] . Commented Oct 22, 2019 at 9:45
  • Any index other than min (or at least, every index i've tested, including the middle element) causes the error to occur. Commented Oct 22, 2019 at 9:49
  • As commented below, it would help to show an actual failing example, along with the calling parameters and the data used. Commented Oct 23, 2019 at 18:45
  • Added some more details and examples. Commented Oct 25, 2019 at 11:31
  • I updated my answer to show a working example of the question's code, but using the middle element for the pivot. Commented Oct 25, 2019 at 13:28

2 Answers 2

1

We can see that at the first step i becomes equal to min, comparison of pivot element with itself fails and increment does not occur more:

int pivot = l[min];
    int i = min - 1;
...
 do{
            i++;
        }while(l[i] < pivot);

Exclude pivot element from comparison (int i = min;) and exchange it with partition one (seems l[j]) at the end

Sign up to request clarification or add additional context in comments.

4 Comments

i = min-1 is the correct way to do this. Take a look at wiki example. With your suggested change, the while loop could advance past the end of the sub-array and fail if there are no values < pivot, such as already sorted data. The initial do while loops depend on the pivot or elements equal to the pivot to stop the loops from running past the ends of the sub-array. Each swap will prevent the next set of do while loops from running off the ends of the sub-array.
@rcgldr I have to agree with you. Was too hurry.
@tobs It is worth to remove accept mark, because your shown code is correct. But you have to show us code that causes errors.
I updated my answer to show the question code using middle element for pivot without issue.
0

Using the middle value for pivot is working for me. Here is a complete example:

    public static void quickSort(int[] l, int min, int max){
        if (min < max){
            int p = partition(l, min, max);
            quickSort(l, min, p);
            quickSort(l, p+1, max);
        }
    }

    public static int partition(int[] l, int min, int max){
        int pivot = l[(min+max)/2];
        int i = min - 1;
        int j = max + 1;
        while(true){
            do{
                i++;
            }while(l[i] < pivot);
            do{
                j--;
            }while(l[j] > pivot);

            if (i >= j) {
                    return j;
            }
            int temp = l[i];
            l[i] = l[j];
            l[j] = temp;
        }
    }

    public static int[] generateRandomArray(int size, int lower, int upper){
        int[] random = new int[size];
        for (int i = 0; i < random.length; i++) {
            int randInt = ThreadLocalRandom.current().nextInt(lower, upper+1);
            random[i] = randInt;
        }
        return random;
    }

    public static void main(String[] args) {
        int[] A = generateRandomArray(10, 0, 9);
        long bgn, end;
        bgn = System.currentTimeMillis();
        quickSort(A, 0, A.length-1);
        end = System.currentTimeMillis();
        for(int i = 1; i < A.length; i++){
            if(A[i-1] > A[i]){
                System.out.println("failed");
                break;
            }
        }
        System.out.println("milliseconds " + (end-bgn));
    }

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.