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

• Andrew Lim: Please note that in this case, the mode is a single mode only. If there are multiple numbers that frequent the same number of times, the mode does not exist.

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.