Coding the QuickSort algorithm - output not quite right

Question

I've never coded this myself before, unfortunately. My implementation operates on a custom class based on sorting the "date" field. Yes I am fully aware I can use the built-in Javascript sort and specify the comparator function but that's not what I'm interested in.

Currently I start with a reverse-sorted list, and then after calling my "target_sort" (QuickSort), I get a not-very-well sorted list.

Code:

function target_sort_wrapper(array) {
    target_sort(array, array.length, 0, array.length);
}


//Quicksort to swap around targets based on dates
//"array" is DDATA, where DDATA[i] are targets
function target_sort(array, length, left, right) {
    if (length < 2) {
        return;
    }
    var pivotIndex = choosePivot(array, length); //returns the index    

    partition(array, pivotIndex, left, right);

    target_sort(array, pivotIndex, 0, pivotIndex - 1);
    target_sort(array, pivotIndex, pivotIndex + 1, array.length);

}

function partition(array, pivotIndex, left, right) {
    //first, put the pivot as the first element to make things easier
    array.swap(pivotIndex, 0);
    var pivot = array[0];
    var i = left + 1;
    for (var j = left + 1; j < right; j++) {
        if (dateValue(array[j].date) < dateValue(pivot.date)) {
            //dateValue converts stuff like "Jun18" into 618, to numerically compare
            array.swap(i, j);
            i = i + 1;
        }
    }
    //don't forget to put pivot back where it belongs
    array.swap(left, i - 1);
}

function choosePivot(array, length) {
    return Math.floor(Math.random() * length); //0 (inclusive) to length (exclusive) 
}

    Array.prototype.swap = function (i, j) {
        var temp = this[i];
        this[i] = this[j];
        this[j] = temp;
        return this;
    }

And here is the output. First the reverse-sorted list is printed, followed by the result of my "target_sort":

Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jun25 Jun25 Jun25 Jun25 Jun25 Jun25 Jun25 Jun25 Jun25 Jun25 Jun25 Jun25 Jun25 Jun25 Jun25 Jun25 Jun25 Jun25 Jun25 Jun25 Jun25 Jun25 Jun25 Jun25 Jun25 Jun25 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 
============================================================= 
Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun25 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun19 Jun25 Jun25 Jun25 Jul05 Jun25 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul05 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jun25 Jul06 Jun25 Jun25 Jun25 Jun25 Jun25 Jun25 Jun25 Jun25 Jun25 Jun25 Jun25 Jun25 Jun25 Jun25 Jul06 Jul06 Jun25 Jul06 Jun25 Jun25 Jun25 Jun25 Jul05 Jun25 Jul05 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul06 Jul05 Jul05 Jul05 Jul06  

I feel like it's getting there, but something is still off.

I've been stuck on this for a few days now, so much thanks for any help.

Cheers.

Answer 1

I have created a correct version now, thank you to Karoly for the hints. Summary:

• Should not do stuff with respect to length, but wrt left and right
• I was always picking a bad pivot for the recursive calls (again, must be wrt left and right, and must fall within the proper range of the subarray)

Code:

function target_sort_wrapper(array) {
    target_sort(array, 0, array.length);
}


//Quicksort to swap around targets based on dates
//"array" is DDATA, where DDATA[i] are targets
function target_sort(array, left, right) {
    if ((right-left) < 2) {
        return;
    }

    var pivotIndex = choosePivot(left, right); //returns the index  

    pivotIndex = partition(array, pivotIndex, left, right);

    target_sort(array, left, pivotIndex);
    target_sort(array, pivotIndex+1, right);

}

function partition(array, pivotIndex, left, right) {
    //first, put the pivot as the first element to make things easier
    var pivot = array[pivotIndex];
    array.swap(pivotIndex, left);
    var i = left + 1; 
    for(var j = left + 1; j < right; j++) {
        //if (array[j] > pivot) { } //do nothing, satisfies invariant
        if (dateValue(array[j].date) < dateValue(pivot.date)) {
        //if (array[j] < pivot) {
            array.swap(i, j);
            i = i + 1; 
        }
    }
    //don't forget to put pivot back where it belongs
    array.swap(left, i-1);
    return (i-1);
}

function choosePivot(left, right) {
    return (left + Math.floor(Math.random() * (right-left)));

}

Array.prototype.swap = function(i, j) {
    var temp = this[i];
    this[i] = this[j];
    this[j] = temp;
    return this;
}

Answer 2

The recursive call is broken:

target_sort(array, pivotIndex, 0, pivotIndex - 1);

One obvious thing is that you pass pivotIndex as length for both partitions which doesn't make any sense.

The other one is that indexing is broken. As you can see the left index is 0, but that clearly won't be true if you on the second recursion level and want to get the left partition of an upper level right partition.

One more thing: the pivot chooser doesn't have to know about the array:

function choosePivot(length)

Hint: Programming is not a guessing game, before you start, decide what your "variables" exactly mean. What's lenght, left, right? For example: Is the right index inclusive (does it point to a part of the partition or just beyond it). Then pick a paper and pencil and figure out the proper indexes and lengths. Trust me, the more carefully you think about it, the quicker you'll finish. Debugging wastes a lot of time. Then, just to verify that you are on the right track use a small toy array for your implementation, and add some console.log messages to see what's going on.