-
-
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 |
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
:
def find_average(values)
sum = 0.0
values.each do |x|
sum += x
end
return sum / values.length
end
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:
def find_average(values)
sum = 0.0
values.each do |x|
sum += x
end
sum / values.length
end
An alternative solution could be written using Enumerable#inject:
def find_average(values)
sum = values.inject(0) { |sum, value| sum += value }
sum / values.length.to_i
end
There's also a shorthand way of using #inject
for simple operations like this:
def find_average(values)
values.inject(&:+) / values.length.to_i
end
You can read more about how that works at http://www.potstuck.com/2011/08/06/ruby-symbols-instead-of-blocks/
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
end
You 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
end
I 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
end
An alternative solution could be written, again using #inject
:
def find_max(values)
values.inject do |max, value|
value > max ? value : max
end
end
Min
Same comments as with #max
Nice job! Please put in a help request if you have any questions about any of my comments.
It's important to spend the extra time to make sure everything is formatted nicely following common ruby idioms. A good reference for doing that is the Ruby Style Guide. There's actually a gem called Rubocop that will run and check your code to find style guide offenses.
Here's some feedback from Rubocop 😄