tailっぽい何かをレビューしてみます。 #Ruby

こちらのコードですが、アルゴリズムとは別の部分で気になる部分がいくつかあったので、まことに僭越ながらレビューしてみたいと思います。とつぜんなので辻斬みたいに感じられたらすみません。

tail_orig.rb

#! env ruby

file_path = ARGV[0]
row_limit = ARGV[1]

def get_offset(file_path, row_limit)
  file = File.open(file_path)
  cr_counter = 0
  offset = 0
  until cr_counter > row_limit.to_i
    offset -= 1
    begin
      file.seek(offset, IO::SEEK_END)
    rescue
      file.seek(0, IO::SEEK_SET)
      break
    end
    cr_counter += 1 if file.read(1) == "\n"
  end
  file.close
  offset + 1
end

def get_tail(file_path, row_limit)
  offset = get_offset(file_path, row_limit)
  file = File.open(file_path)
  file.seek(offset, IO::SEEK_END)
  file.each_line do |line|
    puts line.chomp
  end
  file.close
end

get_tail(file_path, row_limit)

まず全部引用させていただきました。それでは早速気になった部分を指摘していきます。

openはブロック付きで使う

File.open(file_path) do |file|
  # do something
end

File.openはブロックとともに使うことにより、明示的にcloseをする必要がなくなります。closeのし忘れを防げますし、予期せぬ例外が起きた場合にもちゃんとcloseしてくれます。元のコードは例外の起き方によってはcloseされません。リソースリークを防ぐローンパターンと呼ばれるこのパターンが綺麗に書けるのは、rubyの優れた特徴の一つです。

例外は正常フローでは使わない

例外はその名の通り例外的なケースを取り扱うための仕組みです。使う必要のないところでは使うべきではありません。理由は、
* 意図が読み取りにくい。
* 意図せず例外を握りつぶしかねない
* パフォーマンスが悪くなりがち

などがあります。元コードではrescueに例外クラスを指定していません。意図よりも広く例外が拾われているため、貴重な例外情報が発見できず握りつぶされてしまうかもしれません。

この部分はおそらくファイルの先頭より前にシークされることを例外をつかって検出されているのでしょう。この場合はシークする前にファイルのサイズとoffsetを比較すればこの意図は表現できるはずです。

  offset -= 1
  break if offset < -file.size

  file.seek(offset, IO::SEEK_END)

メソッドの引数の型を意識する

get_offsetは引数のrow_limitをStringとして受け取り、row_limit.to_iとして使用しています。しかし、row_limitは行数の上限を表す数字ですから、get_offsetはメソッドの仕様としてNumberを受け取るのが自然でしょう。ARGVから受け取っているので#to_iしているのでしょうが、それはあくまでも呼び出す側の都合です。つまり不必要な結合が生まれています。呼び出す側で#to_iしてから渡しましょう。get_tailメソッドもNumberを受け取るべきなのでその大元で変換してしまいましょう。

row_limit = ARGV[1].to_i

get_fooは返り値を取得するメソッドにつける

get_何々という名前はなにか値を取得するメソッドにつけるべきです。get_offsetはいいと思うのですが、get_tailは副作用に期待したメソッドであり、意味ある値を返していません。print_tailなどでいいのでは。

その他

file.each_line do |line|
  puts line.chomp
end

ここの部分がchompで改行コードを削っておきながら、putsで再度改行を付与する意図がよくわかりませんでした。lineが改行で終わらないケースをケアしてる？

crはcarriage return、日本語では復帰の意味で、"¥r"で表現されます。改行はlf(line feed)で"¥n"ですね。詳しくはhttps://ja.wikipedia.org/wiki/改行コード

修正したコード

最後に全部入りのコードを（別の記事を書く都合でtailコマンドの仕様により近づけてます）

tail.rb

#! env ruby

def get_offset(file_path, row_limit)
  offset = 0
  File.open(file_path) do |file|
    lf_counter = 0
    until lf_counter > row_limit
      offset -= 1
      break if offset < -file.size

      file.seek(offset, IO::SEEK_END)
      chr = file.read(1)
      lf_counter += 1 if (chr != "\n" && offset == -1) || chr == "\n"
    end
  end
  offset + 1
end

def print_tail(file_path, row_limit)
  offset = get_offset(file_path, row_limit)
  File.open(file_path) do |file|
    file.seek(offset, IO::SEEK_END)
    file.each_line do |line|
      print line
    end
  end
end

file_path = ARGV[0]
row_limit = ARGV[1].to_i
print_tail(file_path, row_limit)