What is a more "ruby way" to write this code?

Question

This was a homework assignment for my students (I am a teaching assistant) in c and I am trying to learn Ruby, so I thought I would code it up. The goal is to read integers from a redirected file and print some simple information. The first line in the file is the number of elements, and then each integer resides on its own line.

This code works (although perhaps inefficiently), but how can I make the code more Ruby-like?

#!/usr/bin/ruby -w

# first line is number of inputs (Don't need it)
num_inputs = STDIN.gets.to_i

# read inputs as ints
h = Hash.new
STDIN.each do |n|
  n = n.to_i
  h[n] = 1 unless h[n] and h[n] += 1      
end

# find smallest mode
h.sort.each do |k,v|
  break puts "Mode is: #{k}", "\n" if v == h.values.max
end

# mode unique?
v = h.values.sort
print "Mode is unique: "
puts v.pop == v.pop, "\n"

# print number of singleton odds, 
#       odd elems repeated odd number times in desc order
#       even singletons in desc order
odd_once = 0
odd = Array.new
even = Array.new
h.each_pair do |k, v|
  odd_once += 1 if v == 1 and k.odd?
  odd << k if v.odd?
  even << k if v == 1 and k.even?
end
puts "Number of elements with an odd value that appear only once: #{odd_once}", "\n"
puts "Elements repeated an odd number of times:"
puts odd.sort.reverse, "\n"
puts "Elements with an even value that appear exactly once:"
puts even.sort.reverse, "\n"

# print fib numbers in the hash
class Fixnum
  def is_fib?
    l, h = 0, 1
    while h <= self
      return true if h == self
      l, h = h, l+h
    end
  end
end
puts "Fibonacci numbers:"
h.keys.sort.each do |n|
  puts n if n.is_fib?
end

Answer 1

I don't know if this is a "more Ruby way". At least at is a more "higher-order" way, FWIW.

# first line is number of inputs (Don't need it), thus drop the first line
# read inputs as ints
h = ARGF.drop(1).reduce(Hash.new(0)) {|h, n| h.tap {|h| h[n.to_i] += 1 }}

Not much to say here. Instead of simply looping over ARGF and setting the hash keys, we use reduce to let it do the work for us. And we use a hash with a default value of 0 instead of manually checking the keys for existence.

We use Enumerable#drop to simply drop the first line.

ARGF is a really cool feature stolen (like most of the scripting features) from Perl: if you simply call the script as script.rb without arguments, then ARGF is the standard input. If, however, you call your script like script.rb a.txt b.txt, then Ruby will interpret all the arguments as filenames, open all the files for reading and ARGF will be the concatenation of their contents. This allows you to very quickly write scripts that can take their input either via standard input or a file.

# find smallest mode
modes = h.group_by(&:last).sort.last.last.map(&:first).sort
puts "Mode is: #{modes.first}"

Ruby doesn't have an explicit key-value-pair type, instead most looping operations on hashes use two-element arrays. This allows us to refer to the key and the value with Array#first and Array#last.

In this particular case, we are using Enumerable#group_by to group the hash into different buckets, and the grouping criterion we use is the last method, i.e. the value which in our hash is the frequency. In other words, we group by frequency.

If we now sort the resulting hash, the very last element is the one with the highest frequency (i.e. the mode). We take the last element (the value of the key-value-pair) of that, and then the last element of that, which is an array of key-value-pairs (number => frequency) of which we extract the keys (the numbers) and sort them.

[Note: simply print out the results at every intermediate stage and it's much easier to understand. Just replace the modes = ... line above with something like this:

p modes = h.tap(&method(:p)).
  group_by(&:last).tap(&method(:p)).
  sort.tap(&method(:p)).
  last.tap(&method(:p)).
  last.tap(&method(:p)).
  map(&:first).tap(&method(:p)).
  sort

]

modes is now a sorted array with all the numbers which have that particular frequency. If we take the first element, we have the smallest mode.

# mode unique?
puts "Mode is #{unless modes.size == 1 then '*not* ' end}unique."

And if the size of the array is not 1, then the mode was not unique.

# print number of singleton odds, 
#       odd elems repeated odd number times in desc order
#       even singletons in desc order
odds, evens = h.select {|_,f|f==1}.map(&:first).sort.reverse.partition(&:odd?)

It looks like there is a lot going on here, but it's actually straightforward. You start reading after the equals sign and simply read left to right.

1. we select all items in the hash whose value (i.e. frequency) is 1. IOW, we select all the singletons.
2. we map all the resulting key-value-pairs just to their first element, i.e. the number – IOW we throw away the frequency.
3. we sort the list
4. and then reverse it (for larger lists we should sort in reverse to begin with, since this is quite a waste of CPU cycles and memory)
5. lastly, we partition the array into two arrays, one containing all the odd numbers and the other all the even numbers

6. now we finally look to the left side of the equals sign: Enumerable#partition returns a two-element array containing the two arrays with the partitioned elements and we use Ruby's destructuring assignment to assign the two arrays to two variables

   puts "Number of elements with an odd value that appear only once: #{odds.size}"

Now that we have a list of odd singletons, their number is simply the size of the list.

puts "Elements repeated an odd number of times: #{
  h.select {|_, f| f.odd?}.map(&:first).sort.reverse.join(', ')
}"

This is very similar to the above: select all numbers with an odd frequency, map out the keys (i.e. numbers), sort, reverse and then convert them to a string by joining them together with a comma and space in between.

puts "Elements with an even value that appear exactly once: #{evens.join(', ')}"

Again, now that we have a list of even singletons, printing them is just a matter of joining the list elements with commas.

# print fib numbers in the hash

I didn't feel like refactoring this algorithm to be more efficient and specifically to memoize. I just made some small adjustments.

class Integer

There was nothing in the algorithm that depended on the number being a certain size, so I pulled the method up into the Integer class.

  def fib?

And I dropped the is_ prefix. The fact that this is a boolean method is already explicit in the question mark.

    l, h = 0, 1
    while h <= self
      return true if h == self
      l, h = h, l+h
    end
  end
end
puts "Fibonacci numbers: #{h.keys.sort.select(&:fib?).join(', ')}"

This probably doesn't need much explanation: take the keys, sort them, select all Fibonacci numbers and join them together with commas.

Here is an idea for how to refactor this algorithm. There is a very interesting implementation of Fibonacci using a Hash with default values for memoizing:

fibs = {0 => 0, 1 => 1}.tap do |fibs|
  fibs.default_proc = ->(fibs, n) { fibs[n] = fibs[n-1] + fibs[n-2] }
end

It would look a little like this:

class Integer
  @@fibs = {0 => 0, 1 => 1}.tap do |fibs|
    fibs.default_proc = ->(fibs, n) { fibs[n] = fibs[n-1] + fibs[n-2] }
  end

  def fib?
    i = 0
    until @@fibs[i += 1] > self
      break true if @@fibs[i] == self
    end
  end
end
puts "Fibonacci numbers: #{h.keys.sort.select(&:fib?).join(', ')}"

If anyone can think of an elegant way to get rid of the i = 0, i += 1 and the whole until loop, I'd appreciate it.

Answer 2

Here are some ideas on the first half...

STDIN.gets # just need to get past the first value - not needed, as already noticed

h = Hash.new(0)                    # use the default value argument to Hash#new, 
STDIN.each { |n| h[n.to_i] += 1 }  # so don't have to test for existence

# code seems to be seeking the largest, comment says "smallest"
# Hash#invert switches keys & values, works here if there's only one mode,
# otherwise presents one random key for the modal value
modal_value = h.values.max
mode = h.invert[modal_value]
puts "Mode is: #{mode}"

# OK, so mode may not be unique?
uniqueness = h.select { |k, v| v == modal_value}.size == 1 ? '' : 'NOT'
puts "Mode is #{uniqueness} unique"

#singleton odds ...
singleton_odd_count = h.select { |k,v| v == 1 && k % 2 == 1 }.map { |k, v| k }.size
# ...and evens
singleton_evens = h.select { |k,v| v == 1 && k % 2 == 0 }.map { |k, v| k }
# odd odd counts
odd_odd_count = h.select { |k,v| v % 2 == 1 && k % 2 == 1 }.map { |k, v| k }