Rubyize This: Live in Vancouver. Refactoring #2

written by Scott on January 26th, 2008 @ 08:54 PM

Here’s the second refactoring from the Rubyize This workshop. See the first refactoring for an explanation of what’s going on and why this code is so darn ugly! Don’t forget to check out the third and final refactoring

This script loads the file full of random numbers from the first refactoring and makes a beautiful ascii-art histogram from it.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33

#!/usr/bin/env ruby

require 'pp'

BIG_NUMBER = 32768

# Get data from a file, turn it in to a float, and find the max
data = File.readlines('random.txt')
max = -BIG_NUMBER
for n in (0 ... data.length)
  data[n] = data[n].chomp.to_f
  if data[n] > max
    max = data[n].to_i + 1 # max is ceil(max(data[n]))
  end
end

# Create the empty histogram
histogram = []
for n in (0 .. max)
  histogram.push(0)
end

# Fill the histogram
for n in (0 ... data.length)
  histogram[data[n].to_i] += 1
end

# Print the histogram
pp histogram
puts
for n in (0 .. max)
  puts "*" * histogram[n]
end

Here’s a very concise one-liner from the crowd:



0.upto((data=File.readlines('random.txt').collect {|e| e.chomp!.to_f}).max.to_i) {|i| puts i.to_s + " " + data.select{|a| a==i}.size.to_s}

Here’s Owen’s refactoring:

1
2
3
4
5
6
7
8
9
10
11
12

require 'pp' 
histogram = [] 
File.readlines('random.txt').each do |value| 
  i = value.chomp.to_i 
  histogram[i] ||= 0 
  histogram[i] += 1 
end 

# Print the histogram 
pp histogram 
puts 
histogram.each { | v | puts "*" * v }

Here’s Sam Livingstone Gray’s refactoring

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21

#!/usr/bin/env ruby

require 'pp'

# Get data from a file, turn it in to a float, and find the max

module Enumerable
  def value_counts
    h = Hash.new(0)
    each { |e| h[e] += 1 } 
    h
  end
end

lines = File.readlines('random.txt')
histogram = lines.map { |line| line.chomp.to_i }.value_counts

# Print the histogram
pp histogram
puts
histogram.keys.sort.each { |n| puts '*' * histogram[n] }

I really like this one. Sam created a simple extension to Enumerable that I can see using over and over again.

Here’s what I came up with, with some debugging help from the group

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16

#!/usr/bin/env ruby

require 'pp'

data = File.readlines('random.txt').collect {|datum| datum.chomp.to_f}
max = data.max

histogram = data.inject([0] * (max + 1)) do |histogram, datum|
  histogram[datum.to_i] += 1
  histogram
end

pp histogram
histogram.each do |bucket|
  puts "*" * bucket
end

posted in:

blog

tags:

ruby, rubycamp, rubyize_this

comments are closed

7 comments

Comments

  • anonymous on 26 Jan 16:54

    oneliner trial: 0.upto((data=File.readlines('random.txt').collect {|e| e.chomp!.to_f}).max.to_i) {|i| puts i.to_s + " " + data.select{|a| a==i}.size.to_s}

  • anonymous on 26 Jan 16:55

    oops, should read: 0.upto((data=File.readlines('random.txt').collect {|e| e.chomp!.to_f}).max.to_i) {|i| puts "*" * data.select{|a| a==i}.size}

  • Owen Rogers on 26 Jan 19:53

    there's a bug in this code -- which is why i wasn't able to get it working today. line 19 should read: for n in (0 .. max) if you use the exclusive range operator, it will fail because the histogram array will not initialise all of the necessary values to 0.

  • Owen Rogers on 26 Jan 20:19

    similarly, the exclusive range operator should not used on line 31 either. this is my solution. it could probably be compressed further, but at the expense of readability: require 'pp' histogram = [] File.readlines('random.txt').each do |value| i = value.chomp.to_i histogram[i] ||= 0 histogram[i] += 1 end # Print the histogram pp histogram puts histogram.each { | v | puts "*" * v }

  • Scott on 26 Jan 20:42

    Hmm. You're right, Owen. I guess nobody else had generated random data with numbers in the max bucket. Sorry about that! -- Scott

  • Scott on 26 Jan 20:44

    I've fixed the bug Owen pointed out in the posted code.

  • Sam Livingston-Gray on 26 Jan 21:29

    Here's one of the hash-based solutions. I've never liked the redundancy of doing "foo = Foo.new; foo.do_something; return foo" -- this would look even prettier with ActiveSupport's Object#returning. But I couldn't get it to include properly, so this will do. (= 
    
#!/usr/bin/env ruby

require 'pp'

# Get data from a file, turn it in to a float, and find the max

module Enumerable
  def value_counts
    h = Hash.new(0)
    each { |e| h[e] += 1 } 
    h
  end
end

lines = File.readlines('random.txt')
histogram = lines.map { |line| line.chomp.to_i }.value_counts

# Print the histogram
pp histogram
puts
histogram.keys.sort.each { |n| puts '*' * histogram[n] }

Comments are closed