hi all, i was wondering if my rubyness has increased above the 'total newbie'-level.. so below is some code in my coding style, can you give comments on how it looks and how effective it is. greetings Dirk. def find_factors( number ) factors = [] 1.upto( number ) do | factor | if number%factor == 0 factors << factor end end factors end def find_prime_numbers( max ) factor_list = [] 1.upto( max ) do | number | factor_list << find_factors( number ) end factor_list end def output_factor_list( factor_list , detail=nil ) output = "" if detail factor_list.each_with_index do | factors , number | factors.length == 2 ? output << "*" : output << " " output << (number+1).to_s + ' ' * (factor_list.length.to_s.length-(number+1).to_s.length) + "{ " factors.each do | factor | output << "#{factor} " end output << "}\n" end else output << "{ " factor_list.each_with_index do | factors , number | output << "#{(number+1).to_s} " if factors.length == 2 end output << "}" end output end print output_factor_list( find_prime_numbers( ARGV[0].to_i ) , ARGV[1] )

on 2006-01-29 19:22

on 2006-01-29 20:11

Hi -- On Mon, 30 Jan 2006, Dirk Meijer wrote: > hi all, > i was wondering if my rubyness has increased above the 'total > newbie'-level.. > so below is some code in my coding style, can you give comments on how it > looks and how effective it is. Traditional Ruby indentation is two spaces. One space is a bit of a squeeze :-) When it comes to stylistic things, my favorite source of information is the standard library. You can learn a lot by grepping around. > def find_factors( number ) The space after the opening parenthesis is rare: 1.8$ grep "( " `find . -name "*.rb"` | wc -l 1641 1.8$ grep "(" `find . -name "*.rb"` | wc -l 37951 (and if you filter out 'rexml' and 'yaml' it drops to 544 :-) > factors = [] > 1.upto( number ) do | factor | The space after the first | is non-existent in the standard library: 1.8$ grep "do |" `find . -name "*.rb"` | wc -l 1153 1.8$ grep "{|" `find . -name "*.rb"` | wc -l 1512 1.8$ grep "do | " `find . -name "*.rb"` | wc -l 0 1.8$ grep "{| " `find . -name "*.rb"` | wc -l 0 1.8$ grep "{ | " `find . -name "*.rb"` | wc -l 0 David -- David A. Black dblack@wobblini.net "Ruby for Rails", from Manning Publications, coming May 1, 2006! http://www.manning.com/books/black

on 2006-01-29 20:23

hi, > Traditional Ruby indentation is two spaces. One space is a bit of a > squeeze :-) i actually use tabs, but those didn't seem to be copied properly.. i was actually aiming for comments on the program flow and the way i build up my work, but i'll try to use less whitespace in inapropriate places ;-) greetings, Dirk.

on 2006-01-29 20:25

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

on 2006-01-29 20:53

On Jan 29, 2006, at 1:10 PM, dblack@wobblini.net wrote: >> def find_factors( number ) > > The space after the opening parenthesis is rare: I sure like it though. Makes those argument lists stand out. All my libraries are just like this, but it's true that I don't have anything in the standard library. :) James Edward Gray II

on 2006-01-29 21:11

Jules Jacobs wrote: > Hi Dirk, > [...lot's of stuff i also did :) ...] > 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. What about: -------------------------------------------------------------- def find_factors(number) (1..number).select{|factor| (number % factor).zero?} end def find_prime_numbers(max) (1..max).map{|number| find_factors(number)} end def output_factor_list( factor_list , detail=nil ) if detail (1..factor_list.size).map do |number| (factor_list[number - 1].length == 2 ? '*' : ' ') + number.to_s.ljust(factor_list.length.to_s.length) + '{ ' + factor_list[number - 1].join(' ') + '}' end.join("\n") else "{ "+factor_list.select{|factors| factors.length == 2}.join(' ')+'}' end end puts output_factor_list( find_prime_numbers( 20 ) , true) -------------------------------------------------------------- > 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. It's not that hard, and you will feel like someone cut of your right hand if you have to live without them again. cheers Simon

on 2006-02-03 23:41

Jules, this was very helpful. Thanks for taking the time writing this up! Hans

on 2006-02-04 01:54

Hans, It's nice you found this helpful. I might also, if I had some idea to what you are referring. This posting style, where there is no previous post attached, thus giving a context for your post, is worthless to all but two of you. I'm looking for whatever it was Jules did that so nice. Haven't found it so far. t. On Fri, 03 Feb 2006 14:41:55 -0800, Hans Granqvist <hans@granqvist.com> wrote: > Jules, this was very helpful. Thanks for taking the time > writing this up! > > Hans > -- ================================================ Tom Cloyd, MS MA, LMHC Private practice Psychotherapist Bellingham, Washington, U.S.A: (360) 920-1226 << TC.BestMindHealth.com / BestMindHealth.com >> << tomcloyd@bestmindhealth.com >> ================================================

on 2006-02-13 03:14

Jules Jacobs wrote: > Hi Dirk, >. >. >. > 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 Thanks Jules, Your "coding style" seems very clean, elegant and understandable. I'm very new to Ruby and I've had somewhat of a hardtime getting going with it. I just wanted to say thanks for the detailed explination of what looks to me like ideal ruby code. Hector