PR 6: sort method #6
Conversation
| if array == nil || array.length <= 1 | ||
| return array # nothing to sort |
| swapped = true | ||
| while index < array.length && swapped | ||
| swapped = false | ||
| other_index = index + 1 |
There was a problem hiding this comment.
is there a better variable name for other_index?
| while other_index < array.length | ||
| if array[index] > array[other_index] | ||
| temp = array[index] | ||
| array[index] = array[other_index] | ||
| array[other_index] = temp | ||
| swapped = true | ||
| end | ||
| other_index += 1 | ||
| end | ||
| index += 1 | ||
| end |
| def sort(array) | ||
| if array == nil || array.length <= 1 | ||
| return array # nothing to sort | ||
| end |
There was a problem hiding this comment.
method indentation is misaligned. end can go back one space.
| end | ||
|
|
||
| index = 0 | ||
| swapped = true |
There was a problem hiding this comment.
swapped is an extra variable that is being tracked in this method. the method will still be viable without the swapped variable.
| end | ||
| index += 1 | ||
| end | ||
|
|
There was a problem hiding this comment.
naming convention can be improved. index and other indexes to refer to index position are vague. left and right, or first and last could work better.
| temp = array[index] | ||
| array[index] = array[other_index] | ||
| array[other_index] = temp |
There was a problem hiding this comment.
| temp = array[index] | |
| array[index] = array[other_index] | |
| array[other_index] = temp | |
| array[index], array[other_index] = array[other_index], array[index] |
| while index < array.length && swapped | ||
| swapped = false | ||
| other_index = index + 1 | ||
| while other_index < array.length | ||
| if array[index] > array[other_index] | ||
| temp = array[index] | ||
| array[index] = array[other_index] | ||
| array[other_index] = temp | ||
| swapped = true | ||
| end | ||
| other_index += 1 | ||
| end | ||
| index += 1 | ||
| end |
There was a problem hiding this comment.
Consider reducing the time complexity of O(n^2).
| @@ -0,0 +1,25 @@ | |||
|
|
|||
| def sort(array) | |||
| if array == nil || array.length <= 1 | |||
| if array == nil || array.length <= 1 | ||
| return array # nothing to sort | ||
| end |
There was a problem hiding this comment.
Consider re-writing this as a postfix conditional.
| temp = array[index] | ||
| array[index] = array[other_index] | ||
| array[other_index] = temp |
There was a problem hiding this comment.
have you considered parallel assignment?
array[index], array[other_index] = array[other_index], array[index]
|
|
||
| def sort(array) | ||
| if array == nil || array.length <= 1 | ||
| return array # nothing to sort |
There was a problem hiding this comment.
consider: catching the error that results from the array containing string and number elements. meaning, this edge case throws an error ["sfsf", 324234]
|
|
||
| index = 0 | ||
| swapped = true | ||
| while index < array.length && swapped |
There was a problem hiding this comment.
if possible, consider reducing the O(n^2) time complexity
| while index < array.length && swapped | ||
| swapped = false | ||
| other_index = index + 1 | ||
| while other_index < array.length |
There was a problem hiding this comment.
consider change the nested while loop?
| array[index] = array[other_index] | ||
| array[other_index] = temp |
There was a problem hiding this comment.
consider parallel assignment: array[index], array[other_index] = array[other_index], array[index]
| while index < array.length && swapped | ||
| swapped = false |
There was a problem hiding this comment.
Are you sure this "swapped" guard is working as corrected? I believe it could exit this loop prematurely if array[index] and array[other_index] didn't need to be swapped (since the "true" is only set in the if statement). Have you written tests for this loop?
| if array == nil || array.length <= 1 | ||
| return array # nothing to sort | ||
| end |
There was a problem hiding this comment.
| if array == nil || array.length <= 1 | |
| return array # nothing to sort | |
| end | |
| return array if array == nil || array.length <= 1 |
No description provided.