-
-
Save akcrono/8266bf1bf60c80c17e65 to your computer and use it in GitHub Desktop.
| def find_average scores | |
| sum = 0.0 | |
| scores.each do |x| | |
| sum += x | |
| end | |
| return sum/scores.length | |
| end | |
| def find_max scores | |
| answer = 0 | |
| scores.each do |x| | |
| answer = x if x > answer | |
| end | |
| return answer | |
| end | |
| def find_min scores | |
| answer = scores[0] | |
| scores.each do |x| | |
| answer = x if x < answer | |
| end | |
| return answer | |
| end | |
| scores = [75, 100, 85, 65, 84, 87, 95] | |
| puts find_average scores | |
| puts find_max scores | |
| puts find_min scores |
Max
My first suggestion for this method would be to use the local variable max rather than answer, since this method is looking for the max:
def find_max(values)
max = 0
values.each do |x|
max = x if x > max
end
max
endYou probably don't want to initialize max to 0. If you were to pass in an array of values that were all less than 0, you wouldn't want to return 0 as the max. You could fix this by instead initializing max to be the first item in values:
def find_max(values)
max = values.first
values.each do |x|
max = x if x > max
end
max
endI know you like to optimize things and now you're thinking "oh no! I don't want to iterate over the same value that it's already set to and compare it with itself! What can I do!?":
You can #pop the first value which will remove it from the array and it won't be iterated over later in the #each block:
def find_max(values)
max = values.pop
values.each do |x|
max = x if x > max
end
max
endAn alternative solution could be written, again using #inject:
def find_max(values)
values.inject do |max, value|
value > max ? value : max
end
endMin
Same comments as with #max
Nice job! Please put in a help request if you have any questions about any of my comments.
Average
One thing I'd suggest is using a more generic name for the argument so that the method is more generally applicable to any collection of values rather than just
scores:As said in the ruby style guide that I linked to in the previous comment, in Ruby you should Avoid 'return' where not required for flow controller:
An alternative solution could be written using Enumerable#inject:
There's also a shorthand way of using
#injectfor simple operations like this:You can read more about how that works at http://www.potstuck.com/2011/08/06/ruby-symbols-instead-of-blocks/