Create a gist now

Instantly share code, notes, and snippets.

チームでコードを書き始めた後、「どうやらレビューってやつをした方が良いらしい」くらいの若手に向けた資料です。

コードレビューの基本


一番大事な事

ソースコードはプロジェクトの共同所有物である

  • 誰かだけが触れるコードを無くす
  • 自分だけが持っているコードを無くす
  • 自分だけが触っている時間を短くする

コードレビューで大事な事

コードレビューは...

  • 相互学習型のプロセスである
  • メンバが成長することが大事

相互学習とは

  • レビュアーとレビュイーが、お互い学び合うこと
  • 考え方を共有すること
  • 質問することで学ぼう
  • 一番できる誰かだけが教えるのではない
  • 知識や経験の少ない人が何に躓いているのか学ぼう

メンバの成長

同じミスをチーム内で繰り返さないことが成長


ミスを繰り返さないために

  • 本人の問題にしない
    • 明日はわが身
  • 仕事の正しい手順を覚えよう
  • 道具の正しい使い方を覚えよう

コードレビューの心構え

  • 伝えることが大事
  • 改善するまでがレビュー
  • レビューにコストをかけ過ぎない

コードレビューの心構え

伝えることが大事

  • レビュアーはレビュイーに伝わるように努力しよう
  • できる限り丁寧な表現を心がける
  • 同じ表現で伝わらないなら言い方を変える
  • 感銘を受けたり前回から改善した部分を褒めよう
    • 厳しい事しか言わない人の話を聞き続けるのは難しい
    • 成長に気が付いて貰えると嬉しい

コードレビューの心構え

改善するまでがレビュー

  • 指摘して終わりではなく改善したら終わり
  • 指摘事項の改善を確認するのはレビュアーの責任

コードレビューの心構え

コストとは

  • レビュアーがかける時間
  • レビュアーの人数
  • レビュイーの改善にかける時間
  • レビュアーとレビュイーのメンタル

コードレビューの心構え

レビューのコストについて

  • レビューは改善プロセスであって成果物そのものではない
  • レビューだけやっていても仕事は終わらない
  • レビューはコードを書いてる時間の半分以下を目安に

コードレビューの心構え

コストの低減

  • お互い丁寧に説明し過ぎない
  • 毎回全員が参加する必要はない
  • 完全なものではなく、現時点で妥当なものを選ぶ
  • レビューはメンタルを消耗し易い
    • 特にレビュイーは消耗し易い
    • 被疑者と取調官になってはいけない

コードレビューのやり方

  • 仕様把握
  • 下読み
  • エラー、ワーニング確認
  • 仕様との整合性確認
  • プロジェクト標準との適合性確認
  • レビュー会

コードレビューのやり方

仕様把握

  • レビュー対象となるコードの機能仕様を理解する
  • 仕様を把握せずにレビューするとコードの非常に細かい部分の指摘が増える

コードレビューのやり方

下読み

  • レビュー対象となるコードを全て読む
  • レビュー対象を呼出しているコードも読む
  • レビュー対象が呼出しているコードも読む

コードレビューのやり方

エラー確認

  • コンパイラや静的解析ツールのエラーやワーニングが無い事を確認する
    • ワーニングをSuppressするのは問題ない
    • ワーニングをSuppressしたら理由を説明すること
  • レビュー対象のテストが成功することを確認する
  • ビルドできるコードだけをレビューすること

CIが回っていれば自動化できる


コードレビューのやり方

仕様との整合性確認

  • 仕様通りに動作するコードなのか確認する
  • 自然言語で書かれた仕様書とコードをできる限り一致させる
    • 順序や数、名前など

コードレビューのやり方

仕様との整合性確認例

例えば、以下のような観点で確認するとよい

  • 受け入れるパラメータ
    • 定義順序と数
    • 値の範囲
  • 処理結果として返す値の範囲
  • 処理順序
  • 分岐の評価順序
  • エラー処理
  • 呼出し先に渡すパラメータの範囲

コードレビューのやり方

プロジェクト標準との適合性確認

  • プロジェクトメンバ全員が同じようにコードを書こう
  • 自分達が納得できるやり方でやろう
    • 遠くの偉い人が言ってる事に従ってるだけではダメ
  • まずはeclipseのコードフォーマッタを共有しよう
    • インデント、括弧の位置、等を揃えると読み易くなる

コードレビューのやり方

プロジェクト標準の例

これはあくまでも例であって、自分達に合うやり方を探す事

  • 変数名の長さとスコープの広さを近似する
    • メンバ変数名 > パラメータ名 > ローカル変数名
  • メソッドは一画面に収まるように書く
  • ローカル変数はできるだけ少なく
    • パラメータとローカル変数を併せて7つ程度に収める
  • if文で!(エクスクラメーション)を使わない
    • !は特別なフォントを使わない限り見落とし易いため
    • if(condition == false) と書く
  • 比較演算子は<==だけを使う
  • 例外オブジェクトをなるべく避ける

コードレビューのやり方

レビュー会

  • 基本的にはオンラインでやろう
  • 会議室に集まるのは...
    • ニュアンスが難しい事を指摘するため
    • オンラインでの議論が収束しないとき

まとめ

  • ソースコードはプロジェクトの共同所有物
  • レビューを通して成長しよう

参考資料


最後に

このスライドはMarpで作りました

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment