algorithms-exercises icon indicating copy to clipboard operation
algorithms-exercises copied to clipboard

Update bubble-sort and radix-sort solutions

Open nielsboecker opened this issue 4 years ago • 1 comments

Hi Brian, brilliant course as always, thanks ♡

Bubble sort

Changed index to go only to length - 1. It worked to go up to length, but only implicitly because 10 > undefined === false

    Compare 7 and 8
    Compare 8 and 9
    Compare 9 and 10
    Compare 10 and undefined

Radix sort

The second test in solution works, but only because the input array and the expected output are in fact references to the same array. array.sort() actually performs string-based alphabetic sorting, so this is not what we would want (even though the algorithm is correct).

[1, 10, 100, 1001, 11, 22, 2].sort()
// [1, 10, 100, 1001, 11, 2, 22]

nielsboecker avatar Jun 16 '21 21:06 nielsboecker

This PR appears to be doing more than described in the description. Maybe its worth a clean up? I also discovered the radix sort issue, and came to make a PR for that.

@btholt I see are other possible PRs that address that radix sort but do so without copying the nums array. I feel those are insufficient since radix sort is destructive and thus sort the input array. @nielsboecker solution does copy the array before sorting which I believe is correct.

Other PRs that are testing on an already sorted array (and thus insufficient): https://github.com/btholt/algorithms-exercises/pull/9 https://github.com/btholt/algorithms-exercises/pull/12 https://github.com/btholt/algorithms-exercises/pull/13 https://github.com/btholt/algorithms-exercises/pull/15

marr avatar May 09 '22 13:05 marr