How to make the end of loop more readable in Ruby

Question

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

Answer 1

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.

Answer 2

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.