my coding style

[Dirk Meijer]

------=_Part_8362_9999763.1138558936898
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline

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 =3D []
1.upto( number ) do | factor |
if number%factor =3D=3D 0
factors << factor
end
end
factors
end

def find_prime_numbers( max )
factor_list =3D []
1.upto( max ) do | number |
factor_list << find_factors( number )
end
factor_list
end

def output_factor_list( factor_list , detail=3Dnil )
output =3D ""
if detail
factor_list.each_with_index do | factors , number |
factors.length =3D=3D 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 =3D=3D 2
end
output << "}"
end
output
end

print output_factor_list( find_prime_numbers( ARGV[0].to_i ) , ARGV[1] )

------=_Part_8362_9999763.1138558936898--

 

[dblack]

Hi --

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
(e-mail address removed)

"Ruby for Rails", from Manning Publications, coming May 1, 2006!
http://www.manning.com/books/black

 

[Dirk Meijer]

------=_Part_8758_3815124.1138562417418
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline

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.

------=_Part_8758_3815124.1138562417418--

 

[Jules Jacobs]

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

 

[James Edward Gray II]

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

 

[Simon Kröger]

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

 

[Hans Granqvist]

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

Hans

 

[Tom Cloyd]

Hans,

It's nice you found this helpful. I might also, if I had some idea to wha=
t =20
you are referring. This posting style, where there is no previous post =20
attached, thus giving a context for your post, is worthless to all but tw=
o =20
of you.

I'm looking for whatever it was Jules did that so nice. Haven't found it =
=20
so far.

t.

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

Hans

--=20

=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
Tom Cloyd, MS MA, LMHC
Private practice Psychotherapist
Bellingham, Washington, U.S.A: (360) 920-1226
<< TC.BestMindHealth.com / BestMindHealth.com >>
<< (e-mail address removed) >>
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D

 

[Hector Quiroz]

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