Skip to content

Instantly share code, notes, and snippets.

@akcrono
Created August 18, 2014 01:09
Show Gist options
  • Save akcrono/8266bf1bf60c80c17e65 to your computer and use it in GitHub Desktop.
Save akcrono/8266bf1bf60c80c17e65 to your computer and use it in GitHub Desktop.
first systems check for Launch Academy. 3 methods returning average, max, and min scores.
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
@HeroicEric
Copy link

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 😄

andrew_kostka/list_statistics/systemscheck1.rb:1:18: C: Use def with parentheses when there are parameters.
def find_average scores
                 ^^^^^^
andrew_kostka/list_statistics/systemscheck1.rb:6:3: C: Redundant return detected.
  return sum/scores.length
  ^^^^^^
andrew_kostka/list_statistics/systemscheck1.rb:6:13: C: Surrounding space missing for operator '/'.
  return sum/scores.length
            ^
andrew_kostka/list_statistics/systemscheck1.rb:9:14: C: Use def with parentheses when there are parameters.
def find_max scores
             ^^^^^^
andrew_kostka/list_statistics/systemscheck1.rb:14:3: C: Redundant return detected.
  return answer
  ^^^^^^
andrew_kostka/list_statistics/systemscheck1.rb:17:14: C: Use def with parentheses when there are parameters.
def find_min scores
             ^^^^^^
andrew_kostka/list_statistics/systemscheck1.rb:22:3: C: Redundant return detected.
  return answer
  ^^^^^^

@HeroicEric
Copy link

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/

@HeroicEric
Copy link

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

@HeroicEric
Copy link

Min

Same comments as with #max

@HeroicEric
Copy link

Nice job! Please put in a help request if you have any questions about any of my comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment