Program to find the mean, median, and mode of numbers

Question

I was attempting to build a program using Ruby that asks the user to input at least three numbers, and it returns the mean, median, and mode of those numbers.

A developer friend of mine glanced over it and said that the program was wrong, but didn't state specifically what was wrong with it. I have looked over it and tested it repeatedly and can't figure out what's wrong with it.

puts "Please input three or more numbers with spaces inbetween them:"
numbers = gets.chomp
numbers = numbers.split(" ").map(&:to_i) 
length = numbers.length
y = 0.000
numbers.each do |x|
    y = x + y
end
mean = y / length
print "Mean: #{mean}"
print "\n"
a_order = numbers.sort
length1 = (length - 1) / 2
if length%2 == 1
    median = a_order[length1]
else
    length2 = length1 + 1
    median = (a_order[length1] + a_order[length2]) / 2.000
end
print "Median: #{median}"
print "\n"
frequencies = Hash.new(0)
numbers.each do |number|
    frequencies[number] = frequencies[number] + 1
end
frequencies = frequencies.sort_by { |a, b| b }
frequencies.reverse!
frequencies = frequencies.to_a
if frequencies[0][1] == frequencies[1][1]
    print "Mode: invalid"
else
    print "Mode: #{frequencies[0][0]}"
end

Answer 1

It looks valid to me. Except you are not enforcing the '3 number minimum'.

• You can use x.odd? to check odd or even.
• You can total numbers with [1,2,3].inject(&:+).
• The frequencies.to_a is not needed, as it's already an array after the sort_by.
• You can sort_by -b so you don't have to reverse.

These are all just refactorings. No change in behaviour.

Answer 2

The program is mostly correct. I don't see any reason why this problem requires at least three numbers, and in any case you don't bother enforcing it.

It's a wall of text, though, and rather hard to read as a result. You should definitely organize the code into three functions.

Try to avoid defining a lot of intermediate variables, which introduce clutter. For example,

length = numbers.length
y = 0.000
numbers.each do |x|
    y = x + y
end
mean = y / length

… should be a single expression in a function:

def mean(numbers)
  numbers.inject(:+).to_f / numbers.length
end

---

Similarly,

a_order = numbers.sort
length1 = (length - 1) / 2
if length%2 == 1
    median = a_order[length1]
else
    length2 = length1 + 1
    median = (a_order[length1] + a_order[length2]) / 2.000
end

… should be packed as:

def median(numbers)
  sorted = numbers.sort
  if numbers.length % 2
    sorted[numbers.length / 2]
  else
    0.5 * (sorted[numbers.length / 2 - 1] + sorted[numbers.length / 2])
  end
end

---

In the mode calculation, the counting operation…

frequencies = Hash.new(0)
numbers.each do |number|
    frequencies[number] = frequencies[number] + 1
end
frequencies = frequencies.sort_by { |a, b| b }
frequencies.reverse!
frequencies = frequencies.to_a

… could be done fluently as a single expression.

You have a bug here: if all of the numbers are the same, then frequencies[1][1] will crash.

if frequencies[0][1] == frequencies[1][1]
    print "Mode: invalid"
else
    print "Mode: #{frequencies[0][0]}"
end

def mode(numbers)
  frequencies = numbers.inject(Hash.new(0)) { |h, n| h[n] += 1; h }
                       .sort_by { |n, count| -count }
                       .to_a
  if frequencies.length == 1 || (frequencies[0][1] != frequencies[1][1])
    frequencies[0][0]
  else
    nil
  end
end

---

Once those functions have been defined, the "main" program can be very tidy:

puts "Please input some numbers with spaces in between them:"
numbers = gets.split.map(&:to_i)
puts "Mean: #{mean(numbers)}"
puts "Median: #{median(numbers)}"
puts "Mode: #{mode(numbers) || "invalid"}"

Note that there is no need to chomp if you're just going to split on whitespace anyway.

Printing a string with a newline at the end is better done using puts.