fix: adding median of three technique and module interface to QuickSortRecursive.js and its test file #1686
Conversation
stormkid2009
requested review from
raklaptudirm and
appgurueu
as code owners
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1686 +/- ##
==========================================
+ Coverage 84.65% 84.68% +0.02%
==========================================
Files 378 378
Lines 19744 19791 +47
Branches 2951 2957 +6
==========================================
+ Hits 16715 16760 +45
- Misses 3029 3031 +2 ☔ View full report in Codecov by Sentry. |
| * Sorts the input list using the quicksort algorithm. | ||
| * | ||
| * @param {Array} inputList - the list to be sorted | ||
| * @return {Array} the sorted list |
There was a problem hiding this comment.
Since we mutate the array anyways, it's unnecessary to return it, and IMO cleaner not to.
There was a problem hiding this comment.
You are absolutely right ,thanks for reminding me .
| @@ -43,23 +58,55 @@ const quickSort = (inputList, low, high) => { | |||
| * @returns {number} `pIndex` pivot index value. | |||
| */ | |||
| const partition = (partitionList, low, high) => { | |||
| const pivot = partitionList[high] | |||
| const mid = Math.floor((low + high) / 2) | |||
| // get the median of three is good technique for partitioning to be sure that the 2 sub-arrays will be almost equal or nearly equal in size | |||
There was a problem hiding this comment.
This comment is phrased a bit poorly and also misleading: Nothing is guaranteed here. It's just a heuristic. It can still be horribly wrong if the arrays are crafted to be worst case.
There was a problem hiding this comment.
I am with you that nothing guaranteed but trying to improve something worth it.
|
|
||
| // Move pivot to the end | ||
| let pivotIndex = partitionList.indexOf(pivot) | ||
| ;[partitionList[pivotIndex], partitionList[high]] = [ |
There was a problem hiding this comment.
I would rename some variables to make this less verbose.
Describe your change:
Checklist:
Example:
UserProfile.jsis allowed butuserprofile.js,Userprofile.js,user-Profile.js,userProfile.jsare notFixes: #{$ISSUE_NO}.