コードレビュー

レビューの目的などをまとめてみた：
参考： http://www.ibm.com/developerworks/jp/opensource/library/j_os-codereview/#resources

■目的：
　コードが問題なく単体テストを通っていても、前提とする仕様が誤っていたり誤って解釈していれば、「誤った仕様に沿って正確に動作するコード」となり当初の目的を果たしていない。そもそも単体テストは開発者が作成するのが通常であり、誤って解釈してコーディングされていれば、単体テストも同じ過ちの上に組み立てられいて整合することになり、正しい仕様との乖離は発見できない。
　そこで第3者の目を通してのコードレビューが必要になる。コードレビューは、ソフトウェアレビュー／ソフトウェアインスペクションの一つであり、コード自体を調査することで以下のことを目指す；

障害の可能性を発見：潜在的なバグ(仕様どおりに正しく動作しなくなる可能性)など
コーディングの間違いや勘違いを発見：仕様との乖離
ユーザによる誤操作や誤解を誘発する恐れのある部分を発見：表記、操作順序、UI配置など
管理、運用上での必要データの取得漏れや不適切な保持を発見
性能やりソースに問題をもたらす恐れのある部分を発見：メモリーリーク
基盤(F/W)に対して適合しない部分の発見：認証、認可、セキュリティの迂回ルートなど
将来の保守(修正、拡張など)や移植において問題なる可能性を発見

レビューの過程を通して以下の活動も行われうるが、オプションとする；

テストケースの抜け、漏れの発見と指摘
テスト条件、テストデータの抜け、漏れ、不適切値の発見と指摘
基盤部分の改善や拡張(生産性や保守性が向上する場合)
脆弱性を持つ部分の発見(意図しない情報漏えいや悪意のある操作への対応)

以下の活動は原則として目的としない；これは基盤(アーキテクチャ)や運用環境の課題とする

システムとしてのパフォーマンスチューニング
負荷分散や可用性の向上策
リソースプランニング
秘匿情報の漏洩対策
業務妨害などの攻撃への対策
管理上のファシリティ

■レビューの観点(パースペクティブ)
レビューは種々の観点から行うことで幅広く問題点を指摘することができる；

システム利用者(ユーザ)
システム業務管理担当者
システム運用管理担当者
品質管理担当者

■前提条件：
レビュー開始時点で、ソースが以下の条件を満たしていること；

コンパイルエラーなし
FindBugやCheckStyleでの重大な問題なし
単体テストはAllGreen

■レビューに必要な資料
開発対象概要(業務ユースケース、モジュール構成、データ構成、)
プロジェクト概要(目的、方針、体制)
動作基盤概要(方式、構成、I/F仕様など)
製造要件(要件定義、要件分析、S/W設計、コード表、データ)
製造規約類(コード規約、製造指針、テンプレート、コードテーブルなど)

■レビュー体制
以下の役割で複数人が参画すること；
進行役(モデレータ、司会)
レビューア/インスペクター(問題の指摘；指摘分野を絞る場合もある)
記録係
オーガナイザ (レビュー会開催および結果活用の幹事)
作成者(オーサ、コーダー)
プレゼンター (対象コードの説明役)

■レビュー結果の活用
コードレビューを通して、この種のバグや潜在バグの下地(将来の修正や拡張時にバグを生みやすい構造)を発見して取り除いていくことで、以下の効能がある。

開発者に対して実践的トレーニングをすることになり、技能的向上が見込める
これにより、手戻りが少なくなり、品質、納期、保守性などが向上する

■典型的な指摘例
コードレベルでの指摘
１）変更不可変数の扱い
　必ず使用する変数やフィールドは初期化忘れ防止のためにも「final」宣言をする
　＊不変オブジェクト(生成したら変更不可のもの：String, Integer など)はコンスタント扱いとなる
＝＞ この場合は「static final」が定数の意味合いを強調できるので更によい
　＊不変でないオブジェクトは内容を変更できる
＝＞ フィールドの場合、フィールド定義文またはコンストラクターにて初期化する

２）変数名称
　コードが読み易く、誤解を招かないような名称と長さを保つ

３）スコープの選択
可能な限りスコープを狭くする；他のクラスからの干渉を制限でき、修正や拡張に対する影響を狭くできる；
　内部変数やサブルーチン的なメソッドは「private」
　抽象クラス(abstract)で子クラスが使用するものは「protected」(同じパッケージ内の他クラスからも使える)
　どこからも使えるよう公開する場合は「public」

■基盤部分などの共用コードに対する指摘例
１）スレッド対応
同じデータに複数のスレッドがアクセスする場合、以下の点を考慮する
　ConcurrentXXXX クラスを使用する(Java5以降)
　変数に「volatile」宣言をする
　「synchronized」で囲む：ロックの発生による性能劣化の可能性に注意