Forum: Ruby my coding style

Announcement (2017-05-07): www.ruby-forum.com is now read-only since I unfortunately do not have the time to support and maintain the forum any more. Please see rubyonrails.org/community and ruby-lang.org/en/community for other Rails- und Ruby-related community platforms.
A16652fd5d83c0473bd1e39d9a2117a6?d=identicon&s=25 Dirk Meijer (Guest)
on 2006-01-29 19:22
(Received via mailing list)
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] )
1fba4539b6cafe2e60a2916fa184fc2f?d=identicon&s=25 unknown (Guest)
on 2006-01-29 20:11
(Received via mailing list)
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
A16652fd5d83c0473bd1e39d9a2117a6?d=identicon&s=25 Dirk Meijer (Guest)
on 2006-01-29 20:23
(Received via mailing list)
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.
71f1b6b2c3fd9af2e8c52618fb91caa6?d=identicon&s=25 Jules Jacobs (jules)
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
4299e35bacef054df40583da2d51edea?d=identicon&s=25 James Gray (bbazzarrakk)
on 2006-01-29 20:53
(Received via mailing list)
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
A9c4658e9e475e13d790ae419acf01b6?d=identicon&s=25 Simon Kröger (Guest)
on 2006-01-29 21:11
(Received via mailing list)
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
8b726999a211dfe9c26fa79bf089ba0c?d=identicon&s=25 Hans Granqvist (hgranqvist)
on 2006-02-03 23:41
Jules, this was very helpful. Thanks for taking the time
writing this up!

Hans
2759228660046d2d792700da81544cd0?d=identicon&s=25 Tom Cloyd (Guest)
on 2006-02-04 01:54
(Received via mailing list)
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 >>
================================================
5609f020eec393b57385851485ac692e?d=identicon&s=25 Hector Quiroz (Guest)
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
This topic is locked and can not be replied to.