sinatra_sample.rbのコードレビュー

[igaiga]

https://github.com/YuukiIshibashi/ruby_text_sample/blob/master/sinatra_sample.rb#L4-L8

@posts = []
read_file = File.open("data.txt", "r")
read_file.each {|line|
  @posts << line
}

現状のコードだとFile.openしたあとにcloseする必要があるので、このあとでread_file.closeが必要です。

後述のブロックを使って短く書くとこんな感じになります。

@posts = File.open("data.txt", "r") do |f|
  f.each_line.to_a
end

分かりやすく書くとこんな感じになります。

@posts = []
File.open("data.txt", "r") do |f|
  f.each_line do |line|
    @posts << line
  end
end

https://github.com/YuukiIshibashi/ruby_text_sample/blob/master/sinatra_sample.rb#L19

get_pointメソッド 名前の問題で、このメソッドでやることは「ファイルへポイントとコメントを記録する」だと思うので、メソッド名を save_file とかにすると良いかなと思いました。呼び出し方が save_file(point, comment) になって「ファイルへポイントとコメントを記録する」を表現できるので。また、引き数が2つあると呼ぶ順番を間違えそうなので、次のようにキーワード引き数を使うと呼び出し順を変えても動くので便利です。

save_file(comment: "コメントです", point: 100)
...

def save_file(point:, comment:)
  ...
end

https://github.com/YuukiIshibashi/ruby_text_sample/blob/master/sinatra_sample.rb#L21-L23

現状のコードでも問題ないのですが、次のようにブロックで書くと、ブロックが終わったときに自動でcloseしてくれるので、close忘れのリスクが減って好まれます。とはいえオープンとクローズの概念も学んで欲しいので、最初に現状のコードを見せてから次のコードを見せられると良いかもです。

File.open("data.txt", "a") do |f|
  f.puts(post) 
end

[YuukiIshibashi]

@igaiga なるほど、closeしなければいけなかったのですね・・・ 完全に飛んでました。。 ありがとうございます！修正します

[YuukiIshibashi]

get_pointメソッドについても自分でも違和感があったところでした・・ 修正します！