PR-3 #3
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,19 @@ | ||||||
|
|
||||||
| # Find the maximum element between index 0 and max_index, exclusive | ||||||
|
|
||||||
| def find_max(array, max_index) | ||||||
| max = nil | ||||||
| if max_index == 0 | ||||||
|
There was a problem hiding this comment. Should also check if max_index is nil or not a number There was a problem hiding this comment. How about we check if the array is empty and forego the if/else statement? That way it can print the 'array is empty' statement if it's true and then after the check, we can just set max to array[0]. There was a problem hiding this comment. do you mean:
Suggested change
|
||||||
| puts "Array is empty" | ||||||
|
There was a problem hiding this comment. If max index is 0, maybe we should say the maximum number is equal to the value at that index. |
||||||
| else | ||||||
|
There was a problem hiding this comment. It might make the code a little less nested to have the emptiness check as its own stand-alone function with a short-circuit return and then run the code currently in the "else" portion of the code independently. |
||||||
| max = array[0] | ||||||
|
There was a problem hiding this comment. you should probably check to ensure array has this index first |
||||||
| index = 0 | ||||||
| while index < max_index | ||||||
|
There was a problem hiding this comment. This is not an inclusive search because of the "<" sign. Is this on purpose? There was a problem hiding this comment. if max_index is a negative number, it will not be == to zero but this while loop will also never be true. |
||||||
| if max < array[index] | ||||||
|
There was a problem hiding this comment. max is nil, can't compare nil and a number There was a problem hiding this comment. since you're increasing index until index < max_index is false, you should check that the array has at least max_index number of indices |
||||||
| max = array[index] | ||||||
| end | ||||||
|
There was a problem hiding this comment. Can change lines 11-14 to one line statement: |
||||||
| index += 1 | ||||||
| end | ||||||
| end | ||||||
| return max | ||||||
| end | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is man_index the best name for this var? isn't it actually the array length?