😽
コードレビューをする際の観点について

2024/09/16に公開
 はじめに最近、コードレビューの機会が急増し、それに伴って、コードを読む時間やコメントをする機会が大幅に増えました。その中で、自分なりのレビュー観点が徐々に整理されてきたように感じています。そこで今回は、特に重要だと思うポイントをまとめておきたいと思い、この記事を書くことにしました。
これらの観点を意識してコードを書くことで、極端に悪いコードになるのを防ぐことができます。また、コードレビューを行う方々にとっても参考にしていただけるよう、重要なポイントを整理しました。
それでは、具体的な観点について見ていきましょう。

 観点
 要件を満たすコードを書こうコードが要件を満たしていることは基本中の基本です。しかし、仕様が複雑だったり、頻繁に変更されたりすると、要件を見落としてしまうこともあります。要件に対して正確なコードが書けているか、常に確認することが重要です。
例：
// ユーザーの年齢をチェックし、成人かどうか判定するコード
public boolean isAdult(int age) {
    return age >= 18;  // 要件：18歳以上を成人とする
}
このコードは、「18歳以上を成人とみなす」という要件を満たしています。もし要件が変更された場合は、このロジックも適切に修正する必要があります。

 不要なコード/コメントは削除しよう不要なコードやコメントはコードの可読性を下げ、メンテナンスコストを増やします。運用の経験が薄い方だとピンとこない部分もあるかもしれませんが、デッドコード/デッドコメントが引き起こすメンテナンスコストは想像以上に大きいです。

使われていないコードや役に立たないコメントは積極的に削除しましょう。
例：
public void processOrder(Order order) {
    // オーダーを処理する
    if (order != null) {
        // order.process(); <-- このコードはもう使用されていないので削除すべき

        // 新しい処理
        processOrderNew(order);
    }
}
この例では、古い処理コード order.process(); は使われていないので削除すべきです。残しておくと、後から見た開発者に混乱を招きかねません。

 コードの内容とコメントの内容は一致させようコメントが実際のコードと一致しているか確認しましょう。

コメントとコードの食い違いがあった場合、第三者から見ると、コメントが正しいのかコードの方が正しいのか判断がつきません。

最低でもレビュアーからのQA、最悪の場合は実装ミスのコードが反映されるという事態になってしまいます。
例：
// ユーザーがアクティブかどうかをチェックする（要修正後のコード）
public boolean isActiveUser(User user) {

    // ↓ コメントと実装が食い違っている ↓ 
    // ユーザーの最終ログイン日時が、現在の日付から「一年以内」
    return user.getLastLogin().isAfter(LocalDate.now().minusMonths(6));
}
コメントもコードと一緒に更新しましょう。コードを読むだけで処理が自明な場合はコメントを省略するというのも手段の一つです。

 コメントには意図を書こうコメントには「何をしているか」だけでなく、「なぜそのように実装したのか」という意図を書いておくと、後からコードを読む人が理解しやすくなります。

また先に意図を伝えることで、レビュアーとのQAの件数を減らすこともできます。
// MAX_CACHE_SIZE を 100 に設定する理由：メモリ使用量を最適化し、パフォーマンスを維持するため
// サーバーのメモリ容量やユーザー数に基づいて、100件のキャッシュであればメモリ負荷を抑えつつ、
// 頻繁にアクセスされるデータをキャッシュできると判断しました。
private static final int MAX_CACHE_SIZE = 100;

public void addToCache(String key, Object value) {
    // キャッシュが上限を超えた場合、古いデータを削除して新しいデータを追加する
    if (cache.size() >= MAX_CACHE_SIZE) {
        // 古いエントリを削除
        cache.removeOldestEntry();
    }
    // 新しいエントリをキャッシュに追加
    cache.put(key, value);
}
この例では、MAX_CACHE_SIZE のサイズが「メモリ使用量を最適化し、パフォーマンスを維持するため」という意図をコメントで説明し、具体的な理由として「サーバーのメモリ容量やユーザー数に基づいて100件が適切」と判断したことを記載しています。
こういった意図に関しては、ドキュメントに記載し、コードには書かないという手段もあります。是非はプロジェクトによって分かれるところですが、意図を残しておくという考え方自体はぜひ押さえておいて欲しいところです。

 TODOコメントをただ残したままにしないTODOやFIXMEのコメントは放置せず、必要に応じて速やかに対応しましょう。残す場合は、いつ対処するかも明記しておく、タスクのチケット化をしておくといったようなことをすることで、永遠に放置されるTODOコメントになるという事態を防ぐことができます。
例：
public void calculateDiscount(Order order) {
    // TODO: 複雑なロジックの最適化が必要（次のスプリントで対応: Issue #13 としてIssue化済）
    if (order.isPremium()) {
        order.setDiscount(20);
    } else {
        order.setDiscount(10);
    }
}
この例では、コメントを残すとともにIssueという形で残すことを選択しています。こうすることで後に対処される可能性が大きく上がります。

 おわりに今回まとめた観点について、基本的すぎると感じた方や、期待していた内容と異なると感じた方もいるかもしれません。しかし、実際にはこういった基本的なポイントがコードレビューで指摘されることが多々あります。
コードを書く作業は、常に一定の難易度を伴います。仕様変更への対応やスケジュールのプレッシャーに追われる中で、基本的なことを守るのが難しくなることもあります。さらに、単純に知見が不足しているケースも考えられるでしょう。
こうした観点に基づくレビューは、現状ではまだ生成AIによる完全な自動化が難しく、依然として人の手による確認が必要だと感じています。
システム開発にはコードレビュー含め、多くの課題や困難がつきものですが、共に乗り越えられるよう、お互い引き続き頑張っていきましょう。
はじめに

観点

要件を満たすコードを書こう

不要なコード/コメントは削除しよう

コードの内容とコメントの内容は一致させよう

コメントには意図を書こう

TODOコメントをただ残したままにしない

おわりに

Discussion
