0
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

コードレビューとテストの境界線

0
Posted at

はじめに

すべてをコードレビューで保証しようとした瞬間から、品質は崩れ始めます。

人は複雑な条件分岐を安定して正確に読み続けることができません。
テストで保証できる部分までレビューで抱え込むと、本来見るべき内容を見失います。

品質を守るために必要なのは役割を決めることです。

  • ここまではコードレビューで確認する
  • ここからはテストで確認する

その線を引けるかどうかが品質を分けます。

コードレビューとテストの境界線

障害の多くは、コードレビューが甘かった場所でも、テストが足りなかった場所でもありません。

  • レビューで見ると思っていた
  • テストで拾えると思っていた

そのどちらにも属さなかった「隙間」から生まれます。

必要なのは、レビューを厳しくすることでも、テストを増やすことでもありません。

境界を明確にし、隙間を作らないこと

です。

1.境界

コードレビューで確認

  • 境界条件がコード上に存在するか
  • 比較演算子(<,<=,>,>=,==)が設計どおりか
  • 境界値がハードコードされていないか

テストで確認

  • 境界の内側と外側で結果が切り替わるか
  • 境界値そのもので正しく判定されるか

例:数値の境界

数量が 10以下 の場合のみ処理を行う

if (count <= 10)
{
    Process();
}

コードレビューで確認

  • <= になっているか(< や == ではないか)
  • 境界値が 10 であるか(9 や 11 ではないか)
  • 境界値が直書きで問題ないか(定数化不要か)

⇒ 「境界が正しく書かれているか」

テストで確認

  • count = 10 → 処理される
  • count = 11 → 処理されない
  • count = 9 → 処理される

⇒ 「境界で結果が切り替わるか」

2.入力制御

コードレビューで確認

  • 入力チェックがサーバー側に存在するか
  • UI制御に依存していないか
  • 必須・型・桁チェックが漏れていないか

テストで確認

  • 想定外入力・改ざん入力が拒否されるか
  • UI制御を回避しても安全か
  • エラー内容がユーザーに伝わるか

例:文字列長チェック

名前は 20文字以内

if (name.Length > 20)
{
    throw new ValidationException();
}

コードレビューで確認

  • サーバー側でチェックしているか
  • > 20 になっているか(>= 20 ではないか)
  • UIの maxlength に依存していないか

⇒ 「チェックが存在するか・正しいか」

テストで確認

  • 20文字 → OK
  • 21文字 → エラー
  • UIを改ざんして21文字送信 → エラー

⇒ 「UIを信用していないか」

3.条件分岐

コードレビューで確認

  • 設計にある条件項目がすべて存在するか
  • 不要な条件が混ざっていないか
  • 境界値・比較対象が正しいか
  • 括弧や構造が設計意図を壊していないか

テストで確認

  • 条件成立 / 不成立で結果が切り替わるか
  • AND / OR の誤りが結果として検出できるか
  • 各条件が実際に効いているか

例:複合条件

金額が10,000円以上 かつ(会員 または キャンペーン期間)

if (price >= 10000 && (isMember || isCampaignDay))
{
    ApplyDiscount();
}

コードレビューで確認

  • price >= 10000 が存在するか
  • isMember と isCampaignDay が存在するか
  • 余計な条件(例:isAdmin)が混ざっていないか
  • 括弧が設計どおりか

⇒ 「条件の過不足と構造」

テストで確認

  • 10000円・会員 → OK
  • 10000円・非会員・キャンペーン → OK
  • 10000円・非会員・非キャンペーン → NG
  • 9999円・会員 → NG

⇒ AND / OR の誤りはテストで確実に検出

4.責務の境界

コードレビューで確認

  • 業務ロジックが適切な層にあるか
  • フレームワークやインフラ依存が混ざっていないか
  • テスト不能な設計になっていないか

テストで確認

  • 業務として期待した振る舞いになっているか
  • 外部依存を切り離して確認できるか
  • 責務分離が崩れたときに検知できるか

例:Controller に業務ロジックが混ざるケース

NG
// Controller
public IActionResult Order(int quantity)
{
    if (quantity > 100)
    {
        price *= 0.9m;
    }
}
OK
// Service
public decimal CalculatePrice(int quantity, decimal price)
{
    return quantity > 100 ? price * 0.9m : price;
}

コードレビューで確認

  • 業務ロジックが Controller に書かれていないか
  • Service 層に切り出せるか
  • テストしづらい構造になっていないか

テストで確認

  • 境界値(100)で結果が切り替わるか
  • 単体テストで業務ルールが確認できるか

5.影響範囲

コードレビューで確認

  • 失敗時の影響範囲が想定されているか
  • 例外処理・ロールバック方針があるか
  • ログ・通知設計があるか

テストで確認

  • 失敗時に業務が継続できるか
  • 想定外の影響が出ていないか
  • 問い合わせにつながる挙動かどうか

例:例外処理

try
{
    SaveOrder(order);
}
catch (Exception ex)
{
    logger.Error(ex);
}

コードレビューで確認

  • 例外を握り潰していないか
  • ログが出るか
  • 業務継続可否が設計されているか

テストで確認

  • DBエラー時に画面はどうなるか
  • 業務が継続できるか
  • 問い合わせが発生する挙動か

6.変化耐性

コードレビューで確認

  • 「今はこうだから」という前提がコードに埋め込まれていないか
  • 仕様が変わったときに、同じ修正を何か所もしなくてよいか
  • 条件が増えたときに if 文が爆発しない構造か

テストで確認

  • 日付跨ぎ・データ増加時の挙動
  • 設定変更時の影響
  • 回帰テストとして残す価値があるか

例:日付判定

if (orderDate < DateTime.Today)
{
    Reject();
}

コードレビューで確認

  • DateTime.Now ではなく Today を使っているか
  • 時刻を含める / 含めない設計が明確か
  • テスト不能なコードになっていないか

テストで確認

  • 昨日 → NG
  • 今日 → OK
  • 日付跨ぎ(23:59 → 00:00)

7.デグレ履歴

コードレビューで確認

  • 過去障害への対策がコードに残っているか
  • なぜその処理が必要か読み取れるか
  • リファクタで消されやすくないか

テストで確認

  • 過去障害ケースが再現できるか
  • 同じ不具合が再発しないか
  • 修正意図がテストとして固定化されているか

例:過去障害対応コード

// 過去にNULLで障害が出たため
if (customer == null)
{
    return;
}

コードレビューで確認

  • なぜこの処理があるか分かるか
  • 削除してよいコードではないか
  • コメントや履歴が残っているか

テストで確認

  • customer が null の場合に落ちないか
  • 過去障害が再発しないか

おわりに

コードレビューは
設計の通りに書かれているかを見る工程

テストは
設計の通りに動いているかを見る工程

境界を明確にし隙間をなくすこと。
それが品質設計です。

0
0
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
0
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?