Bas Velden
Bas Velden

Reputation: 428

Why do these quick sort algorithms not work fully?

I wrote my own version of quick sort which seemed to work with the hard coded array I gave it. Then I tested it with arrays randomly generated containing positive and negative numbers. Then I noticed that some times the elements are not sorted correctly.

I then pasted another algorithm from here, which gave the same results. Then I pasted another from the answer posted here. I then used an iterative version from here

The code I have now (this code is from the second link posted above):

public static void QuickSortRecursive(int[] arr, int left, int right) 
{
    if (left > right || left < 0 || right < 0) 
        return;

    int index = Partition(arr, left, right);

    if (index != -1) 
    {
        QuickSortRecursive(arr, left, index - 1);
        QuickSortRecursive(arr, index + 1, right);
    }
}

private static int Partition(int[] arr, int left, int right) 
{
    if (left > right) 
        return -1;

    int _left = left;

    int pivot = arr[left];    
    for (int i = left; i < right; i++) 
    {
        if (arr[i] < pivot) 
        {
            swap(arr, i, _left);
            _left++;
        }
    }

    Swap(arr, _left, right);

    return _left;
}

private static void Swap(int[] A, int left, int right) 
{
    int tmp = A[left];
    A[left] = A[right];
    A[right] = tmp;
}

When running this with this array: [66, -27, 49, -19, 91, 4, -8, -99, 15, 83]

It outputs: [ -99, 15, -27, 49, -19, 4, -8, 83, 66, 91 ]

I know I must be missing something obvious...however, I can't figure it out.

Upvotes: 2

Views: 259

Answers (2)

rcgldr
rcgldr

Reputation: 28828

Fixes noted in comments:

    public static void QuickSortRecursive(int[] arr, int left, int right) 
    {
        if (left >= right)          // fix
            return;
        int index = Partition(arr, left, right);
        // no need to check index == -1  
        QuickSortRecursive(arr, left, index - 1);
        QuickSortRecursive(arr, index + 1, right);
    }

    private static int Partition(int[] arr, int left, int right) 
    {
        //                          // removed the if
        int pivot = arr[left];
        int _left = left+1;                     // fix
        for (int i = _left; i <= right; i++)    // fix
        {
            if (arr[i] < pivot) 
            {
                Swap(arr, i, _left);
                _left++;
            }
        }
        _left--;                    // fix
        Swap(arr, _left, left);
        return _left;
    }

    private static void Swap(int[] A, int left, int right) 
    {
        int tmp = A[left];
        A[left] = A[right];
        A[right] = tmp;
    }

Upvotes: 1

Bas Velden
Bas Velden

Reputation: 428

As I thought I missed something obvious. When printing the sorted array to the console, I actually printed the same array as the one returned from an iterative version (the implementation from the 3rd link posted above). When I used the code from the SO answer posted in the 2nd link, this gave the wrong output because that implementation is actually incorrect (even though it is accepted and upvoted several times).

So basically only the first link has a fully working implementation. while the other 2 oddly enough are incorrect. And with the first link I printed the same array twice (from the iterative implementation) causing me to think both implementations were incorrect.

Upvotes: 0

Related Questions