This is IT

技術、日常

【プログラム初心者必見】プロによるレビュー内容のまとめ

概要

　現在、私はフィヨルドブートキャンプというスクールでお世話になっています。 ここでメンターをしてらっしゃる、伊藤淳一さんからプロ目線でのコードレビューをして頂いたので、自分の後学のためにもまとめておきたいと思います。

　なお、実際にプラクティス中の課題で受けたレビューになりますので、内容はある程度抽象化して記載していきます。 また、プラクティスの中でも序盤の方の課題となるため、レベル的にはプログラミング未経験～初学者向けの内容になるかと思います。

  • 変数名の付け方
  • コメントの付け方
  • その他

変数名の付け方

複数形の単語で終わる変数名は、「配列」として捉えられることが多い

　複数形で終わる変数名は、「配列」として認識されることが一般的に多いようです。 右辺は単なる数値を返すメソッドにも関わらず、左辺の変数名がそのようになっている場合には気を付けましょう。

# BAD
lines = hoge.length

# GOOD
line_char_length = hoge.length


変数名の末尾の単語に意識を向ける

　上記に付随した指摘になります。

　変数名は基本的に、末尾の単語を見ることで、どのような型のオブジェクトが代入をされているのかを予測します。

max_char_finename = hoge.max_by(&:length).length

　上記の例だと、左辺の末尾の単語を見るとfilenameとなっているので、「ファイル名を代入しているのかな？」と予測されてしまいます。

　右辺のメソッド通り、ファイル名の長さを今回は代入をしたいので、

max_filename_length = hoge.max_by(&:length).length

のようにコードを書いてあげた方がより良いものとなります。

スコープが短くても変数名は適当に付けない

　スコープが短いと、arrやtmpなどのいわゆる何の意味ももたない変数名を使いがちです。

　もちろん、iなどの変数名を使う場面もあるのですが、何の意味も持たない変数名は付けないほうが吉です。(伊藤さんは「無味無臭」な変数名と名付けています)

after_XXXing_YYYの変数名は、XXXed_YYYに変える

　『XXXをした後のYYY』という変数名を付けたい場合は、単純にXXXを過去形にした方が短い変数名になります。

# BAD
after_transposing_files

# GOOD
transposed_files


コメントの付け方

見ればわかるコメントを書かない

　コードを見ればわかる事をコメントに書いてしまうと、余分なノイズになってしまいます。基本的には"WHY"に答えているようなコメントを記述してあげるのが良いです。

qiita.com

主観的なコメントを避ける

　人によって見方が変わる主観的なコメントは避けましょう。人によって捉え方が変わってきてしまうので、こちらが意図したものとは違う認識をされてしまう恐れがあります。

# データが大きすぎると処理が遅くなるため、ここで一度このような処理をしました。
# 綺麗に表示をさせるため、左揃えにしました。

　例えば「データが大きすぎると」→「データが100MBを超えてしまうと」のように数値のような客観的な指標を利用してコメントをした方が誤認識を生む可能性は下がるでしょう。

　また、そもそも本当にコメントを付ける必要があるのか？という考えも大事です。「綺麗に～」の部分は主観的かつ、どのような表示をさせたいのかはコードを見れば十二分に分かるため、そもそもこのコメント自体が必要ありませんでした。

その他

mainメソッドを定義する

　これは目から鱗でした。多分今後もこのような書き方を自分はしていくと思います。ぜひ記事を読んでみてください。

qiita.com

「falseとnil以外は全て真」であることを意識する

　意外と忘れがちです。このことを意識すると、条件分岐などはもっと短く書けるかもしれません。

　例えばある配列のnilのみを除外して、表示させる処理ではこの考えを利用してリファタクリングが可能です。

foo = [1, 2, nil, 3, nil]

# BAD
foo.delete(nil)
foo.each{|int| p int} #=>1 2 3

# GOOD
foo.each do |int|
  p int if int
end


メソッドを飛び越えるためだけのインスタンス変数は避ける

　僕みたいな初心者はやりがちかもしれません。「他のメソッドでも同じ変数を使いたいからインスタンス変数にしちゃえ～！」はよくありません。

　メソッドの戻り値は最後に評価された式のみを戻り値にしてくれます。そのため、一つのメソッド内でいくつもの処理を行おうとしすぎると、使いたくなる戻り値が増え、インスタンス変数を使いたくなる問題が生じやすいです。

qiita.com

無限ループを生むリスクを避ける

　whileやuntilは無限ループを生む可能性があるので、避けるのが吉です。 timesメソッドを出来たら使いましょう。

# BAD
sum += 1 while foo

# GOOD
hoge = 数値
hoge.times { sum += 1}


破壊的メソッドを避ける

　言わずもがなです・・・。ただ「！が付いていると破壊的」なんて覚え方をしてしまっていると、罠にはまります。 　例えば<<メソッドも破壊的なので気を付けましょう。

# BAD
files << nil

# GOOD
files += Array.new[]