プログラミングにおいてJavaDoc/コードコメントの効果的な書き方について

このページに書いてあること

JavaDocには呼び出す側が知りたい契約を、コードコメントには実装を選んだ理由を残します。利用者IDから表示名を探す例で、書きすぎる場合、書かない場合、契約に絞る場合を比べます。

最もやばいと思うJavaDoc=詳細設計書パターン

実装の各行を説明している当時のJavaDocです。利用者を探すという同じ処理でも、検索方法や分岐の順番を変えるたびに、呼び出し側の契約とは関係のない説明まで修正が必要になります。

やばいポイント

途中の処理を別メソッドに移すだけでJavaDocの説明が古くなります。一方、呼び出す側に必要な「IDが未入力ならエラー」「見つからなければ空」という条件は、手順の中から探すことになります。

書かなすぎて何しているかわからない問題パターン

「処理を実行して結果を返す」とだけ書かれていても、どのIDを受け付けるか、未入力と検索結果なしをどう区別するかは判断できません。公開メソッドの契約として、両方を区別できる文言を残します。

やばいポイント

検索結果なしだけを想定して画面を作ると、未入力で投げられた例外を処理できません。実装を全部読む必要がある状態は、呼び出し側の修正漏れにつながります。

個人的に思う、こういうコメントを書いて欲しい

「利用者ID」「表示名」のように、基本設計で使う語をメソッド名とJavaDocに揃えます。処理の流れは当時の図のように、入力確認、検索、結果を返す順です。

JavaDocには入力・返却値・例外条件を記載します。コードの近くに置くコメントは、端末がIDの両端へ空白を付ける場合がある、という実装理由の1箇所です。検索先を変更しても、契約が同じ限りJavaDocはそのままです。

import java.util.Optional;
import java.util.function.Function;

public class UserLookup {
    private final Function<String, Optional<String>> search;

    UserLookup(Function<String, Optional<String>> search) {
        this.search = search;
    }

    /**
     * 利用者IDに対応する表示名を返す。
     *
     * @param userId 利用者ID。前後の空白は除く。
     * @return 利用者が存在しない場合は空のOptional
     * @throws IllegalArgumentException 利用者IDがnullまたは空白の場合
     */
    public Optional<String> findDisplayName(String userId) {
        if (userId == null || userId.isBlank()) {
            throw new IllegalArgumentException("userId is required");
        }
        // 連携元の端末は、ID欄の両端に空白を付ける場合がある。
        return search.apply(userId.strip());
    }
}

import java.util.ArrayList;
import java.util.List;
import java.util.Optional;

public class UserLookupCheck {
    public static void main(String[] args) {
        var calls = new ArrayList<String>();
        var lookup = new UserLookup(id -> {
            calls.add(id);
            return id.equals("U-001") ? Optional.of("Aki") : Optional.empty();
        });

        check(lookup.findDisplayName(" U-001 ").equals(Optional.of("Aki")));
        check(lookup.findDisplayName("U-999").isEmpty());

        for (String missing : new String[]{null, "  "}) {
            try {
                lookup.findDisplayName(missing);
                throw new AssertionError("missing ID accepted");
            } catch (IllegalArgumentException expected) {
                check(expected.getMessage().equals("userId is required"));
            }
        }

        check(calls.equals(List.of("U-001", "U-999")));
        System.out.println("5 checks passed");
    }

    private static void check(boolean condition) {
        if (!condition) {
            throw new AssertionError();
        }
    }
}

javac UserLookup.java UserLookupCheck.java
java UserLookupCheck
# 5 checks passed

まとめ

コードレビューでは、JavaDocだけを読んで未入力、該当なし、該当ありの扱いを決められるかを確認します。実装コメントには、検索方法が変わっても知っておきたい前提を残します。