2
\$\begingroup\$

There are some blocks in my code.

To make it easier to read, I often append comment after the end

such as

  end # end upto
  end # fileopen

Because sometimes the indentation still not easy for read.

Is there any better practice?

And my indentation is 2 spaces, is it OK for most Rubier ?

require "faker"
real_sn = 2124100000
File.open("lib/tasks/tw_books.txt", "r").each do |line|
  # author = Faker::Lorem.words(2..5).join(' ').gsub('-','').gsub('\s+','\s')

  1.upto(1000).each do |x|

    location = Faker::Lorem.words(5)
    book_name, author, publisher, isbn, comment = line.strip.split("|||")
    ap(line)
    isbn = isbn[/\d+/]
    real_sn+=1
    bk = Book.new(:sn => real_sn,:name => book_name, :isbn=>isbn,
                  :price =>Random.rand(200..5000), :location=>location, :category=>["商業","歷史","體育","政治"].sample,
                  :author => author, :sale_type => [:fix_priced, :normal, :promotion].sample, :publisher => publisher,
                  :release_date => rand(10.years).ago, :comment => comment
                  )
    if bk.save()
      if (real_sn%100)==0
        puts book_name
      end
      Sunspot.commit
    else
      puts real_sn
      puts bk.errors.full_messages
    end
  end # end upto
end # fileopen
\$\endgroup\$
5
  • 3
    \$\begingroup\$ Refactor for readability. The trailing comments are irritating and do nothing for readability, they only detract. \$\endgroup\$ Oct 31, 2013 at 0:04
  • 1
    \$\begingroup\$ @DaveNewton: Trailing end comments are irritating for blocks shorter than ~10 lines, and valuable for long blocks, such as module and class definitions. Fortran authors were not completely insane for making the END "comments" mandatory. \$\endgroup\$
    – Boris Stitnicky
    Oct 31, 2013 at 7:20
  • \$\begingroup\$ @BorisStitnicky If a block is that long it's almost always missing one or more refactorings. Use normal docs at the beginning of classes and modules for visual separation. When we wrote Fortran we wrote in a very different style-and it's not a language feature that survived as we moved forward. \$\endgroup\$ Oct 31, 2013 at 11:06
  • \$\begingroup\$ @DaveNewton: ... module and class definitions ... \$\endgroup\$
    – Boris Stitnicky
    Oct 31, 2013 at 21:22
  • \$\begingroup\$ @BorisStitnicky ... They're not valuable. \$\endgroup\$ Oct 31, 2013 at 21:33

2 Answers 2

6
\$\begingroup\$

Compare your code to how I'd write it:

require "faker"

real_sn = 2124100000

File.open("lib/tasks/tw_books.txt", "r").each do |line|
# author = Faker::Lorem.words(2..5).join(' ').gsub('-','').gsub('\s+','\s')

  1.upto(1000).each do |x|

    location = Faker::Lorem.words(5)
    book_name, author, publisher, isbn, comment = line.strip.split("|||")

    ap(line)

    isbn = isbn[/\d+/]
    real_sn += 1

    bk = Book.new(
      :author => author,
      :category =>["商業","歷史","體育","政治"].sample,
      :comment => comment,
      :isbn => isbn,
      :location =>location,
      :name => book_name,
      :price => Random.rand(200..5000),
      :publisher => publisher,
      :release_date => rand(10.years).ago,
      :sale_type => [
        :fix_priced,
        :normal,
        :promotion
      ].sample,
      :sn => real_sn,
    )

    if bk.save()

      puts book_name if ((real_sn % 100) == 0)
      Sunspot.commit

    else

      puts real_sn
      puts bk.errors.full_messages

    end
  end
end

Part of writing code is making it readable and maintainable. That means use indentation, vertical alignment, whitespace between operators, vertical whitespace to make changes in logic more obvious, etc.

I sort hash keys alphabetically, such as the parameters for Book.new, especially when there's a lot of them. This makes it a lot easier to see if something is duplicated or missing. Since it's a hash it doesn't matter what order they're in as far as Ruby is concerned; Again this is for maintenance later on.

The editor you use can help you immensely with this. I use gvim and Sublime Text, both of which allow me to easily reformat/reindent code I'm working on, and I take advantage of that often. It's a good first step when you have code from a "foreign source" that makes your eyes bug out. Reindent it, fix long, awkward sections, like your list of hash entries for Book.new, and the code will become more understandable.

Also, your editor needs to have the ability to jump between matching delimiters like (), [], {} and do/end. gvim can do that plus jump through if/else/end plus rescue blocks. That ability to navigate REALLY helps keep your code flow clear.

\$\endgroup\$
2
  • \$\begingroup\$ I noticed that you left the comments at the end of the end statements. Is that really how you would write this, or did you just leave it in? \$\endgroup\$
    – vgoff
    Oct 31, 2013 at 4:33
  • \$\begingroup\$ No, I wouldn't use comments on end. If I have to do that to figure out logic or control structures I haven't written it clearly enough up above. Those were there as left-overs; I was mostly showing everything above the final close of the blocks. I'll take them out for thoroughness. \$\endgroup\$ Oct 31, 2013 at 6:11
1
\$\begingroup\$

To provide an alternative to the respected Tin Man's answer, commenting ends is a practice that I use when the block body is long (yours is longish). When the body is short, commenting ends is unnecessary. I would write your code as:

require "faker"
real_sn = 2124100000

File.open( 'lib/tasks/tw_books.txt', ?r ).each do |line|
  # author = Faker::Lorem.words( 2..5 ).join( ' ' ).gsub( '-', '' ).gsub( '\s+', '\s' )
  1.upto( 1000 ).each do |x|
    location = Faker::Lorem.words( 5 )
    book_name, author, publisher, isbn, comment = line.strip.split( "|||" )
    ap( line )
    isbn = isbn[ /\d+/ ]
    real_sn += 1
    bk = Book.new( sn: real_sn, name: book_name, isbn: isbn,
                   author: author, publisher: publisher,
                   location: location,
                   category: %w[ 商業 歷史 體育 政治 ].sample,
                   sale_type: %i[fix_priced normal promotion].sample,
                   release_date: rand( 10.years ).ago,
                   price: Random.rand( 200 .. 5000 ),
                   comment: comment )
    if bk.save then
      puts book_name if real_sn % 100 == 0
      Sunspot.commit
    else
      puts real_sn
      puts bk.errors.full_messages
    end
  end # end upto
end # File.open

You can notice a few mainly cosmetic changes, such as colon notation in keyword: arguments, spaces inside parenthesis (I find it more readable), word array and symbol array literals...

Reusable code is write once, read many times. When writing reusable code, one should make it brief, but not by using abbreviations of common words, or by cramming characters together. Conversely, when writing one-time code, it is OK to use abbreviations and save keystrokes. I hope that you do not reuse your code example much. If you do, you should refactor it into 2 or 3 smaller methods, or even method object(s).

The editor you use can help you immensly. Matz uses Emacs. In fact, he was inspired by Emacs when creating Ruby. Emacs gives one everything expected of a programming editor. In fact, Emacs gives you power, for the price of having to actually read the manual. With Emacs, I use yasnippet for code snippets, and auto-complete-el as the most intelligent auto completion agent. One other modification that makes life in Emacs easy is redefining Caps Lock to be a Ctrl key.

Apart from that, the Ruby style manual that I found valuable is the one by Ian McDonald from Google.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.