From: Jules Jacobs <julesjacobs@...>
Date: 2006-01-30T04:25:23+09:00
Subject: Re: my coding style

Hi Dirk,

I have translated your code a bit. You can make it much shorter if you 
use blocks:

This method selects all factors of a number. Here, I first created a 
list of values from 1 to n (1..n). Then only the factors are selected 
(the non-factors are filtered).

def find_factors(n)
 (1..n).select{|factor| n % factor == 0}
end

This method creates a list of numbers from 1 to max (1..max). Then it 
replaces each number n in the list with find_factors(n). That is what 
map does:

def find_prime_numbers(max)
 (1..max).map{|n| find_factors(n)}
end

The output method is still very tricky, and not elegant. I first changed 
factor_list.length.to_s.length to Math.log10(list.length).ceil. Then I 
moved it out of the loop, because it is more efficient (it will only be 
computed once, and not again in every iteration). I think you know what 
log10 is? Well, log10 and then ceil (round up) returns the number of 
numbers in a number. So 23 has 2 numbers, and 1234 has 4.

Then I replaced the output variable with inject, and I made some minor 
changes like:

factors.length == 2 ? output << "*" : output << " "

to:

output + (factors.length == 2 ? '*' : ' ')

And

factors.each do | factor |
    output << "#{factor} "
end

to:

factors.join(' ')

and in non-detail mode, I used select and join again.

def output_factor_list(list, detail = false)
    if detail
        spaces = Math.log10(list.length).ceil
        number = 0
        list.inject('') do |output, factors|
            number += 1
            output + (factors.length == 2 ? '*' : ' ') +
            number.to_s + (' ' * (spaces - Math.log10(number + 1).ceil)) 
+
            '{ ' + factors.join(' ') + " } \n"
        end
    else
        '{ ' + list.select{|factors| factors.length == 2}.join(' ') + ' 
} '
    end
end

Everything combined:

def find_factors(n)
 (1..n).select{|factor| n % factor == 0}
end

def find_prime_numbers(max)
 (1..max).map{|n| find_factors(n)}
end

def output_factor_list(list, detail = false)
    if detail
        spaces = Math.log10(list.length).ceil
        number = 0
        list.inject('') do |output, factors|
            number += 1
            output + (factors.length == 2 ? '*' : ' ') +
            number.to_s + (' ' * (spaces - Math.log10(number + 1).ceil)) 
+
            '{ ' + factors.join(' ') + " } \n"
        end
    else
        '{ ' + list.select{|factors| factors.length == 2}.join(' ') + ' 
} '
    end
end

print output_factor_list(find_prime_numbers(200), false)

I think most of the changes are from imperative to functional. 
Inject/map/select /join really make things simpler, but they are hard to 
understand if you haven't used them.

I hope this was helpful,

Jules

-- 
Posted via http://www.ruby-forum.com/.