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

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

• 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. Jul 28 '13 at 11:20

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.

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.