# レビュー指導において

## なぜ、コードレビューが必要か
そもそも受講生は、レビューが何か？レビューが何故必要か？を理解していない場合があります。
その際は、レビューの必要性について指導しましょう。

### レビューとは

### レビューの必要性

[](https://xtech.nikkei.com/it/article/COLUMN/20080523/303901/)
>　一つは，質的（品質）な進捗の把握である。レビューをすると，機能の漏れや抜け，不具合などがある程度分かる。特に，その分野の有識者や顧客が参加するレビューには効果がある。上流工程で品質が確保できるので，手戻りを削減でき，生産性向上にもつながる。

### レビューのタイミング
いつでも。

### レビューの方法

## レビュー観点

### 全般
- [x] コードが動作する
- [x] コードが仕様を満たしている
(理解度が低い受講生さんはそもそもここができていない可能性がある)
- [x] コードを動作するための手順が明確
(特に、Webアプリ開発においては、なんとなくやっている場合は、どのようにサーバに反映されるかを確認。ホットデプロイの説明)
- [x] 不要なコードがない
(コメントの消し忘れのチェック)
- [ ] 重複するコードがない
- [ ] 標準APIにある機能を独自実装していない
- [ ] 広く使われている外部ライブラリを独自実装していない
- [ ] 長過ぎるコードがない
- [ ] 1つの関数は20行まで(個人的には、10行にしてもいいくらい)
- [ ] TBD(クラスの大きさなどなど)

### コーディング規約
- [ ] 統一されたコーディングスタイルで書かれている
(統一されていても特殊な場合は、言語のデファクトスタンダードなコーディング規約を紹介)
- [ ] コードに意思が感じられる
(受講生がなにかキレイにコードを書こうとする意思があるかの確認。リーダブルコードなどの書籍の紹介)

### フォーマット
- [ ] インデントを正しくつける

### 変数
- [ ] 適当な変数名をつけない
- [ ] 英語でつけている
辞書を引くように推奨
参考サイトの紹介
- [ ] スペルが正しい
- [ ] `camelCase`, `snake_case`が統一されている
- [ ] クラス定数や、スコープの広い定数には`CONSTANT_CASE` を利用している
- [ ] 単数形、複数形の使い分けができている
- [ ] `tmp`、`buffer`など、汎用的過ぎる命名をしていない
- [ ] 汎用的な命名をしている場合は、それ相応の理由がある
- [ ] `name1`, `name2`などとしている場合は、リストにする、`first_name`・`last_name`や`parent_name`・`child_name`などのように具体的な命名にできない状態である
- [ ] スコープの広い変数に略語を用いていない。
(使用可能な略語と使用シーンのリストを共有。それに違反しているものはNG。)
- [ ] 変数に意味を込められている
- [ ] 限界値を示す変数に min_ max_を使っているか
- [ ] 範囲を示す変数に first_ last_を使っているか
- [ ] 短縮しない
(例外があるなら、そのリストアップを運営側でしておく。)
- [ ] 変数のスコープを必要最低限にする
- [ ] 不要な変数が定義されていない
- [ ] 定数ではなく変数の場合、正当な理由がある
- [ ] 変数に再代入する際は、その再代入が適切である(他の変数を用意するべきではないか？)
- [ ] 判断しづらいマジックナンバーが書かれていない
(マジックナンバー自体は容認するが、その値が容易に推察できない場合は、変数にさせる。)

### データ型
- [ ] 適切なデータ型を使えている(特に、リストとマップの比較)

### 関数
- [ ] 関数化を適切に行えている
- [ ] 早期リターンの推奨
- [ ] 関数名は動詞になっている。なっていない場合は、それ相応の理由がある
- [ ] VOなどのように英語の語順を意識できている
- [ ] 戻り値の型が予想しやすい命名になっている
- [ ] getXXXXメソッドをアクセッサー以外で定義していないか
(例えば、getSumのような命名をされることがあるので、calculateSumなどに変更する。)
- [ ] 1つの関数は1つの処理だけを行っている
### クラス
- [ ] 責務や意味を持ったクラスにできている
- [ ] 変数やメソッドが適切なクラスに属している
- [ ] 変数やメソッドの可視性が適切である
- [ ] 結合度が適切できある

### コメント
- コメントアウトが残っていない
- コードに書いていることをそのまま書いていない
- JavaDoc, docStringなどが書けている

### セキュリティ
- [ ] パスワードが埋め込まれていない
- [ ] パスワードが一方向ハッシュ化されている
- [ ] ハッシュ化のアルゴリズムが適切である
- [ ] 秘密鍵などが直接記述されていない
- [ ] ユーザから受け取った文字列をそのまま利用していない
- [ ] 入力バリデーションを行っている
- [ ] PreparedStatementなどを利用して、SQLインジェクションなどの攻撃に対する対策を行えている
- [ ] HTMLにレンダリングする際は、エスケープする関数など言語やフレームワーク、テンプレートエンジンの機能を利用している

### ファイル
- 絶対パスを使用していない

### リソース
- 適切にクローズしている
- try-with系の構文がある言語では、それが利用できている

### より良くするために
- [ ] `sum = sum + ??? -> sum += ???` のように複合代入演算子が使えている
(必須ではないが、このパターンは大体、複合代入演算子をそもそも理解していないケースが多いので、要指導)
- [ ] lenやsize系の関数・メソッドの利用している
(IndexOutOfBoudsException的なものが起こるなど、理由付きで説明)
- [ ] forEach系の構文の利用できている
(IndexOutOfBoudsException的なものが起こるなど、理由付きで説明)
- [ ] 無駄な計算の省略ができている

    for score in scores.values():
      total += score
      ave = total / len(scores)

例えば、上記だと、平均点の計算はfor文の外でOK
- [ ] 2段以上のネストがない
(関数化の練習も兼ねて、2段以上のネストはすぐに関数化するようにする。)
- [ ] テンプレートリテラルやフォーマット文字列系の利用ができている
- [ ] 疑似乱数を適切に使えている(毎回インスタンス化していないか？)
- [ ] ログ出力レベルが適切である

### その他気になったことメモ
- パスについての理解
- データ型の理解

### 講師の指導について
- 答えを教えるのではなく、理由を理解してもらう
- 状況を整理する質問をする
`xxx`という変数はどのような値ですか？など