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
200_success
146k22 gold badges190 silver badges478 bronze badges
asked Oct 31, 2013 at 0:02
\$\endgroup\$
5
  • 3
    \$\begingroup\$ Refactor for readability. The trailing comments are irritating and do nothing for readability, they only detract. \$\endgroup\$ Commented 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\$ Commented 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\$ Commented Oct 31, 2013 at 11:06
  • \$\begingroup\$ @DaveNewton: ... module and class definitions ... \$\endgroup\$ Commented Oct 31, 2013 at 21:22
  • \$\begingroup\$ @BorisStitnicky ... They're not valuable. \$\endgroup\$ Commented 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.

answered Oct 31, 2013 at 0:24
\$\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\$ Commented 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\$ Commented 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.

answered Oct 31, 2013 at 7:05
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.