Skip to main content
added 693 characters in body
Source Link
konijn
  • 34.4k
  • 5
  • 71
  • 267

Interesting question,

as far as I can tell, quicksort is recursive, your solution is not. Hence, your solution is not really quicksort.

Other than that, this makes me cringe:

  var pivotIndex = undefined;
  var low_Index = undefined;
  var high_Index = undefined;

  pivotIndex = inputArray.length - 1;
  low_Index = 0;
  high_Index = (inputArray.length > 2) ? (inputArray.length - 2) : 0;

just declaring a variable assigns the value undefined, there is no reason to assign it, so don't do it. Then right after you declared those variables, you assign values, which you could also do in the var statement. Finally, and this is truly a matter of tastes you could chain those statements:

  var pivotIndex = inputArray.length - 1,
      low_Index = 0,
      high_Index = (inputArray.length > 2) ? (inputArray.length - 2) : 0;

Even here, you could reflect on the repetition of inputArray.length and assign this to a shorter variable name, to make this more readable, and a fraction faster:

  var length = inputArray.length,
      pivotIndex = length - 1,
      low_Index = 0,
      high_Index = (length > 2) ? (length  - 2) : 0;

Then again, you are not using a consistent naming standard, always use lowerCamelCase in JavaScript. I think some professors dislike ternaries, I would avoid them in school or at least check with the professor.

  var length = inputArray.length,
      pivotIndex = length - 1,
      lowIndex = 0,
      highIndex = Math.max(length - 2, 0);

Other than that your white-spacing is far too generous, never use more than a double newline.

This probably also makes you loose points:

for(var low = low_Index; low < pivot_Index; low++){

I would use index instead of low, otherwise reading your code becomes too hard.

Finally, also check your code for repetition, you swap two entries a number of times, I would have written a dedicated function for that.

Interesting question,

as far as I can tell, quicksort is recursive, your solution is not. Hence, your solution is not really quicksort.

Other than that, this makes me cringe:

  var pivotIndex = undefined;
  var low_Index = undefined;
  var high_Index = undefined;

  pivotIndex = inputArray.length - 1;
  low_Index = 0;
  high_Index = (inputArray.length > 2) ? (inputArray.length - 2) : 0;

just declaring a variable assigns the value undefined, there is no reason to assign it, so don't do it. Then right after you declared those variables, you assign values, which you could also do in the var statement. Finally, and this is truly a matter of tastes you could chain those statements:

  var pivotIndex = inputArray.length - 1,
      low_Index = 0,
      high_Index = (inputArray.length > 2) ? (inputArray.length - 2) : 0;

Other than that your white-spacing is far too generous, never use more than a double newline.

This probably also makes you loose points:

for(var low = low_Index; low < pivot_Index; low++){

I would use index instead of low, otherwise reading your code becomes too hard.

Finally, also check your code for repetition, you swap two entries a number of times, I would have written a dedicated function for that.

Interesting question,

as far as I can tell, quicksort is recursive, your solution is not. Hence, your solution is not really quicksort.

Other than that, this makes me cringe:

  var pivotIndex = undefined;
  var low_Index = undefined;
  var high_Index = undefined;

  pivotIndex = inputArray.length - 1;
  low_Index = 0;
  high_Index = (inputArray.length > 2) ? (inputArray.length - 2) : 0;

just declaring a variable assigns the value undefined, there is no reason to assign it, so don't do it. Then right after you declared those variables, you assign values, which you could also do in the var statement. Finally, and this is truly a matter of tastes you could chain those statements:

  var pivotIndex = inputArray.length - 1,
      low_Index = 0,
      high_Index = (inputArray.length > 2) ? (inputArray.length - 2) : 0;

Even here, you could reflect on the repetition of inputArray.length and assign this to a shorter variable name, to make this more readable, and a fraction faster:

  var length = inputArray.length,
      pivotIndex = length - 1,
      low_Index = 0,
      high_Index = (length > 2) ? (length  - 2) : 0;

Then again, you are not using a consistent naming standard, always use lowerCamelCase in JavaScript. I think some professors dislike ternaries, I would avoid them in school or at least check with the professor.

  var length = inputArray.length,
      pivotIndex = length - 1,
      lowIndex = 0,
      highIndex = Math.max(length - 2, 0);

Other than that your white-spacing is far too generous, never use more than a double newline.

This probably also makes you loose points:

for(var low = low_Index; low < pivot_Index; low++){

I would use index instead of low, otherwise reading your code becomes too hard.

Finally, also check your code for repetition, you swap two entries a number of times, I would have written a dedicated function for that.

Source Link
konijn
  • 34.4k
  • 5
  • 71
  • 267

Interesting question,

as far as I can tell, quicksort is recursive, your solution is not. Hence, your solution is not really quicksort.

Other than that, this makes me cringe:

  var pivotIndex = undefined;
  var low_Index = undefined;
  var high_Index = undefined;

  pivotIndex = inputArray.length - 1;
  low_Index = 0;
  high_Index = (inputArray.length > 2) ? (inputArray.length - 2) : 0;

just declaring a variable assigns the value undefined, there is no reason to assign it, so don't do it. Then right after you declared those variables, you assign values, which you could also do in the var statement. Finally, and this is truly a matter of tastes you could chain those statements:

  var pivotIndex = inputArray.length - 1,
      low_Index = 0,
      high_Index = (inputArray.length > 2) ? (inputArray.length - 2) : 0;

Other than that your white-spacing is far too generous, never use more than a double newline.

This probably also makes you loose points:

for(var low = low_Index; low < pivot_Index; low++){

I would use index instead of low, otherwise reading your code becomes too hard.

Finally, also check your code for repetition, you swap two entries a number of times, I would have written a dedicated function for that.