現場で経験したコードレビューのポイントなどを書いておこうかと思います。
コードについてはみな日頃書いているものでもあり、神学論争になりやすいネタでもあるのであくまで参考として。
ある程度コードを書いてきて、コードレビューに関する知見をざっくりまとめたい、などの時に見返していただければと思います。
自動チェック項目（ツール対応推奨）
規約的なことですね。
こういったものをいちいち人が指摘していては、時間と労力の無駄になりますので、静的解析ツールなどを入れて、チェックするようにしましょう。
またCIなどでプッシュ時に自動検知される仕組みを構築し、ルールを守らないとマージできない環境を整えましょう。
ただコードを書くメンバーからするとコミット時に気づくのでは遅過ぎるため、同一の規約やルールをエディタに読み込ませ、規約違反時にすぐに気づけるようにしましょう。
文字のキャメル、スネークの統一、その他ルール
ツールで検知できるため、これで回避しましょう。
スコープの長さと変数名を対応させるなども、コードを書いていて癖がつく部分もありますが、人が見ると見落としが起こりますので、ツールで。
if文やfor文のスペースや括弧の位置
癖をつけておけば大丈夫（目が違和感を覚える）だと思いますが、100%になることはなかなか難しいので、ツールでチェックできるようにしましょう。
行の長さ、メソッドの長さ、クラスの長さチェック
1行あたりの場合はチェックがつくことが多い（100文字以下など）ですが、メソッド、クラスの長さなどはチェックができないことが多いでしょう。
この点は分割することを癖にするのが一番良く、規約云々ではなく、短い方が書きやすくなることが多いです。
ただ例外もあるので、キツくし過ぎると運用時に負担になってしまうことも多いように思われます。
人的チェック項目（レビュアー対応）
一般的にここからがコードレビューなどで話題に上がる箇所かと思います。
自分が悩んだり、現場で色々議論の対象となった点などを記しておきたいと思います。
コードレビューのポイントですが、具体的な指摘事項よりも背景にある考え方や思想の方が大事だと思います。
以前の現場などで中心となっていた思想などについてまとめてみたいと思います。
命名
- 品詞や粒度が統一されているか、統一するような仕組みができているか
- 冗長すぎないか、抽象的すぎないか
- メソッドに関して規則があるか（get、setなどの頻出のものを多用しすぎていないか）
- 一般的な命名ルールに準拠しているか
責務の分離
- そのクラスやメソッドで実行しようとしている責任や役割(責務)というものがしっかり定義されており、適度な単位に分割されているか
- なるべく小さい単位で分割されているか
- 適切に責務に分離されていれば、Controllerでもない限り、メソッドが長くなることはないはず
- 各クラス分けが適切か（Controller、Service、Modelなど）
- Controller、Service、Modelの仕事がそれぞれ1つずつか（色々な仕事をさせていないか）
可読性
- 引数が多すぎないか、数は適切か
- 1つのメソッドで1つの責務になっているか(複数の仕事をさせていないか)
- 引数に必要以上に配列を使っていないか（ブラックボックスになりがちなので、少なければ直接引数を。多ければそれ用のObjectを。）
- 定数などを定義Enumなどで置き換えているか（他人が見たときにコードがわかりやすいか）
- 配列を使いすぎていないか（役割を持ったオブジェクトに置き換える、分けるなど）
型のチェック
- DBに入れる場合に型チェックしたものを使っているか。setterの方がfillなどで一気に値を入れるよりも値がしっかり担保される
- nullのある場合、可能性を常に考慮しているか
- getter,setterなどでこれらの値の保証ができているか
- 不十分なメソッドや条件判定（PHPでいうところの3重イコールではなく、2重イコールを使っていないか。またemptyなどを使っていないか。正しくはis_nullなど。）
特定メソッドの推奨
- JavaではStringBuilderで文字を追加する、Listの定義ではArrayListやHashListを使わないなど
- 言語ごとの作法やルールに習熟しているか
コメントの書き方
- Document作成ツールでコメントから起こせる場合もあるので、しっかり記載する
- コード内部のコメントに関してはなるべく書かない(書かないくらいわかりやすく可読性を向上させるべき)という意見も
抽象化
- コードが適切に抽象化されているか（共通のものが多い場合は親を作り、親子関係ができているか、あるいは親子の関係が適切か）
- 抽象化されすぎて、逆に可読性が落ちていないか
- 例 会員情報などを登録するメソッドなどは引数をmembersとするより、個別の名前、性別、年齢などを引数にもったほうが可読性が向上する
保証された値を使っていないか
- requestパラメータはバリデーションを通過したもののみを使う
- 何らかのfilterを通過した値を使っているか
負荷
- 大量のデータを一括で取得していないか（chunkなどで小出しにとる、分割するなど）
- 画面の場合はデータ量を考えて設計されているか
- レスポンス時間が考慮されているか
- そもそも使用するミドルウェアが正しいか（DBでなくNoSQLを使うなど）
その他
- フレームワークの標準の機能などを使っているか
- テストコードが適切か
- プロジェクト内のコーディングの各種ルールを守っているか
効果的なレビュー体制の構築
コードレビューを実運用する場合の運用について、現場でネックだったことやその取り組みなどを少し書かせていただきます。
定型的なものはチェックリストを活用
何度も書きましたが、なるべく頭を使わずに自動ではじける仕組みが大切です：
- ドキュメントの体裁→チェックリストやなるべく標準のテンプレート、標準の修正機能で防げるようにしたい
- 規約的なものは例えば規約ツールで自動ではじくなど
このレベルで毎回指摘する、指摘事項が変わるなどがないようにしたいものです。
レビュアーの負担軽減
- 仕様が複雑だったり、粒度が大きいものは対面でまずざっくりとレビュイーが説明をし、改善点に関しては回覧レビュー（メッセージのやり取りだけ）を行う
- 当事者意識を持たせるために、レビュアーにノルマを付与するといいかも（一回のレビューの際に必ず何らかの指摘事項をもうけるなど）
- リマインダーなどで自分の分を告知する仕組みが必要かも（Slackへの告知など）
- 参加する際の前提知識やそもそもの仕様の共有が最も大切
- そもそもの人間関係を壊さないような文化づくり（根本的にある信頼関係の構築）が最も大切
- 神学論争的なものは最終決定権のある人間に決めてもらう（当人同士で争わないように）
レビューポイントの最適化
- 見るポイントを各人で担当制にする（仕様的な部分、コードの設計的な部分）
- できればファイルなどを細かく分割して、小分けにする(一般論としてすくなければすくないほどよい)
- 必ず直す、直してほしい、できたら、のようにウェートをつける（理想論でいえば全て直してほしいが、そのタスクがブロックになってしまい、後続タスクへの影響などをなくすため。できたら・・などは別タスクとして別途レビューをする）
- 指摘事項で多いもののリストアップ→なるべく他のメンバーでもレビューをできるようにする、見れるようにするため
- 余裕があればレビューの指摘事項で上がっているものの類型化など