0

I have been trying to implement a quickSort function and have everything working. But there is one peculiarity that I cannot wrap my head around or understand why.

In this first block of code, you will see that I have declared some default param values for the quickSort() function:

function swap(arr, firstIndex, secondIndex) {
  let temp = arr[firstIndex];
  arr[firstIndex] = arr[secondIndex];
  arr[secondIndex] = temp;
}

function pivot(arr, start = 0, end = arr.length - 1) {
  // We are assuming that the pivot is always the first element
  let pivot = arr[start];
  let swapIndex = start;

  for (let i = start + 1; i <= end; i++) {
    if (pivot > arr[i]) {
      swapIndex++;
      swap(arr, swapIndex, i);
    }
  }

  // Swap the starting element with the pivot index
  swap(arr, start, swapIndex);
  return swapIndex;
}

function quickSort(arr, left = 0, right = arr.length - 1) {
  if (left < right) {
    let pivotIndex = pivot(arr, left, right);
    // left
    quickSort(arr, left, pivotIndex - 1);
    // right
    quickSort(arr, pivotIndex + 1, right);
  }
  return arr;
}

In this example, it works fine as expected. However, if I were to remove the ES2015 default param values from quickSort() and, instead, create the defaults inside of the function, like this:

function quickSort(arr, left, right) {
  left = left || 0;
  right = right || arr.length -1;
  if (left < right) {
    let pivotIndex = pivot(arr, left, right);
    // left
    quickSort(arr, left, pivotIndex - 1);
    // right
    quickSort(arr, pivotIndex + 1, right);
  }
  return arr;
}

I get an infinite loop/Stack Overflow issue and I cannot understand why. From what I can tell, the issue is caused by the third param - right - rather than the left param, as the code works fine if I call the left param using the pre-es2015 method, whilst leaving the right param with the ES2015 param default method.

All in all, I have my code working, so that's fine - I just want to try and better understand why this would cause an issue, as I've never encountered such a problem before.

Thanks!

Sunil Sandhu
  • 1
  • 1
  • 13
  • if you pass `0` to the third parameter, it gets changed to `arr.length - 1` in the second version - not so in the first version. The default value is use when no argument is passed - your code uses a default when the argument is anything falsey (like 0) ... to write the same code as ES2015, you need to check the length of the arguments, not the value – Jaromanda X Mar 09 '20 at 20:30

2 Answers2

2

The problem is that your two versions work differently when 0 is passed as right. (And because that's a base case, you get an infinite loop).

right = right || arr.length -1;

evaluates to the right hand side when 0 is passed in, as 0 is falsy.

The default parameter initialiser on the other hand puts 0 in right if 0 is passed in, and evaluates the arr.length - 1 expression only when undefined (or no argument at all) is passed in.

To replicate that behaviour, in ES5 you'd write

function quickSort(arr, left, right) {
  if (left === undefined) left = 0;
  if (right === undefined) right = arr.length - 1;
  if (left < right) {
    let pivotIndex = pivot(arr, left, right);
    // left
    quickSort(arr, left, pivotIndex - 1);
    // right
    quickSort(arr, pivotIndex + 1, right);
  }
  return arr;
}
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
0
const quicksort = ([x, ...rest]) => (!x || x === undefined) ? [] : quicksort(rest.filter(a => a < x))
.concat(x)
.concat(quicksort(rest.filter(a => a >= x)))`

Sorry this is ecmascript6 and not es5.

Dan
  • 59,490
  • 13
  • 101
  • 110
ptothep
  • 415
  • 3
  • 10