PR-3 #3
PR-3 #3
Conversation
| if max_index == 0 | ||
| puts "Array is empty" | ||
| else | ||
| max = array[0] |
|
|
||
| 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
|
|
||
| def find_max(array, max_index) | ||
| max = nil | ||
| if max_index == 0 |
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].
| else | ||
| max = array[0] | ||
| 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?
| max = nil | ||
| if max_index == 0 | ||
| puts "Array is empty" | ||
| 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.
| def find_max(array, max_index) | ||
| max = nil | ||
| if max_index == 0 | ||
| 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.
| if max_index == 0 | ||
| puts "Array is empty" | ||
| else | ||
| max = array[0] |
There was a problem hiding this comment.
you should probably check to ensure array has this index first
| else | ||
| max = array[0] | ||
| index = 0 | ||
| while index < max_index |
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.
| max = array[0] | ||
| index = 0 | ||
| while index < max_index | ||
| if max < array[index] |
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
|
|
||
| # Find the maximum element between index 0 and max_index, exclusive | ||
|
|
||
| def find_max(array, max_index) |
There was a problem hiding this comment.
Is man_index the best name for this var? isn't it actually the array length?
|
|
||
| def find_max(array, max_index) | ||
| max = nil | ||
| if max_index == 0 |
| while index < max_index | ||
| if max < array[index] | ||
| max = array[index] | ||
| end |
There was a problem hiding this comment.
Can change lines 11-14 to one line statement:
max = array[index] if max < array[index]
| else | ||
| max = array[0] | ||
| index = 0 | ||
| while index < max_index |
|
|
||
| def find_max(array, max_index) | ||
| max = nil | ||
| if max_index == 0 |
|
|
||
| def find_max(array, max_index) | ||
| max = nil | ||
| if max_index == 0 |
There was a problem hiding this comment.
do you mean:
| if max_index == 0 | |
| if array.length == 0 |
| max = array[0] | ||
| index = 0 | ||
| while index < max_index | ||
| if max < array[index] |
There was a problem hiding this comment.
max is nil, can't compare nil and a number
| def find_max(array, max_index) | ||
| max = nil | ||
| if max_index == 0 | ||
| puts "Array is empty" |
| if max_index == 0 | ||
| puts "Array is empty" | ||
| else | ||
| max = array[0] |
| else | ||
| max = array[0] | ||
| index = 0 | ||
| while index < max_index |
This method find the maximum value of an array.