my coding style

Discussion in 'Ruby' started by Dirk Meijer, Jan 29, 2006.

  1. Dirk Meijer

    Dirk Meijer Guest

    ------=_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--
    Dirk Meijer, Jan 29, 2006
    #1
    1. Advertising

  2. Dirk Meijer

    Guest

    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


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

  3. Dirk Meijer

    Dirk Meijer Guest

    ------=_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--
    Dirk Meijer, Jan 29, 2006
    #3
  4. Dirk Meijer

    Jules Jacobs Guest

    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/.
    Jules Jacobs, Jan 29, 2006
    #4
  5. On Jan 29, 2006, at 1:10 PM, 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
    James Edward Gray II, Jan 29, 2006
    #5
  6. 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
    Simon Kröger, Jan 29, 2006
    #6
  7. Hans Granqvist, Feb 3, 2006
    #7
  8. Dirk Meijer

    Tom Cloyd Guest

    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.

    On Fri, 03 Feb 2006 14:41:55 -0800, Hans Granqvist <> =20
    wrote:

    > 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 >>
    << >>
    =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, Feb 4, 2006
    #8
  9. 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

    --
    Posted via http://www.ruby-forum.com/.
    Hector Quiroz, Feb 13, 2006
    #9
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Paul Baxter

    style for coding latches

    Paul Baxter, Aug 10, 2003, in forum: VHDL
    Replies:
    7
    Views:
    1,133
    Mike Treseler
    Aug 15, 2003
  2. Willem Oosthuizen

    Coding style to prioritize certain inputs

    Willem Oosthuizen, Sep 2, 2003, in forum: VHDL
    Replies:
    5
    Views:
    495
    Mike Treseler
    Sep 4, 2003
  3. Analog Guy

    Coding style for CPLD vs FPGA

    Analog Guy, Mar 11, 2005, in forum: VHDL
    Replies:
    3
    Views:
    5,445
    Klaus Falser
    Mar 14, 2005
  4. calmar
    Replies:
    11
    Views:
    808
    calmar
    Feb 21, 2006
  5. Ken Varn
    Replies:
    0
    Views:
    444
    Ken Varn
    Apr 26, 2004
Loading...

Share This Page