Writing system software: code comments.

Salvatore Sanfilippo

システムソフトウェアを書く:コードコメント

しばらく前から、YouTubeの「writing system software」シリーズでコードコメントについて語る新しい動画を撮りたいと思っていました。しかしよく考えてみると、このテーマは動画よりもブログ記事の方が向いていると気づき、こうして記事を書くことにしました。この記事ではRedisのコメントを分析し、分類を試みます。その過程で、なぜ私がコメントを書くことが良いコードを作るうえで極めて重要だと考えているのかを示していきます。長期的に保守可能で、他の人はもちろん、修正やデバッグの際に作者自身にとっても理解しやすいコードを作るためにです。

誰もが同じように考えているわけではありません。コードが十分にしっかりしていればコメントは不要だと考える人も多くいます。すべてがよく設計されていれば、コード自体が何をしているかを物語ってくれるのだから、コードコメントは余計だという考え方です。私はこの見方には、主に二つの理由から賛成できません。

  1. 多くのコメントは、コードが何をしているかを説明しているのではありません。コードが何をしているかだけを見ても分からないことを説明しているのです。多くの場合、そこで欠けている情報とは、なぜコードがある特定の動作をしているのか、あるいは、なぜより自然に思える別のやり方ではなく、はっきりと書かれているその方法を取っているのか、という理由です。
  2. コードが何をしているかを行ごとに逐一説明することは、読めば分かるのですから一般的には有用ではありません。しかし、読みやすいコードを書くうえでの重要な目標の一つは、読者がコードを読む際に頭の中に抱えなければならない労力や詳細の量を減らすことです。ですから私にとってコメントは、読者の認知負荷を下げるための道具となり得るのです。

次のコードスニペットは、上記の2点目の良い例です。なお、この記事に掲載するすべてのコードスニペットはRedisのソースコードから引用しています。どのスニペットも、抜粋元のファイル名を先頭に付けて掲載しています。使用したブランチは現在の「unstable」で、ハッシュは32e0d237です。

scripting.c:
    /* Initial Stack: array */
    lua_getglobal(lua,"table");
    lua_pushstring(lua,"sort");
    lua_gettable(lua,-2);       /* Stack: array, table, table.sort */
    lua_pushvalue(lua,-3);      /* Stack: array, table, table.sort, array */
    if (lua_pcall(lua,1,0,0)) {
        /* Stack: array, table, error */

        /* We are not interested in the error, we assume that the problem is
         * that there are 'false' elements inside the array, so we try
         * again with a slower function but able to handle this case, that
         * is: table.sort(table, __redis__compare_helper) */
        lua_pop(lua,1);             /* Stack: array, table */
        lua_pushstring(lua,"sort"); /* Stack: array, table, sort */
        lua_gettable(lua,-2);       /* Stack: array, table, table.sort */
        lua_pushvalue(lua,-3);      /* Stack: array, table, table.sort, array */
        lua_getglobal(lua,"__redis__compare_helper");
        /* Stack: array, table, table.sort, array, __redis__compare_helper */
        lua_call(lua,2,0);
    }

LuaはスタックベースのAPIを採用しています。上の関数の各呼び出しを、Lua APIのリファレンスを手元に置きながら追えば、読者はいかなる時点のスタックの状態も頭の中で再現できるでしょう。しかし、なぜ読者にわざわざそんな労力を強いる必要があるのでしょうか。コードを書いた元の作者は、書く際にいずれにせよその精神的な労力を払っているのです。私がそこでやったのは、各呼び出しの後にその時点のスタックの状態を一行ごとに注記しただけです。これでこのコードを読むのは、Lua API自体は決して追いやすいとは言えないにもかかわらず、ごく簡単になりました。

ここでの私の目的は、ソースコードの局所的な部分だけを読んでも明らかにはならない背景を提供する手段としてのコメントの有用性について、自分の見解を示すだけではありません。歴史的に無用あるいは有害とさえ見なされてきた種類のコメント、すなわち、なぜではなく何をしているかを述べるコメントが有用であることについても、証拠を示すことです。

コメントの分類

この作業を始めたとき、私はRedisのソースコードのランダムな部分を読み、異なる文脈でコメントが有用かどうか、なぜ有用なのかを確かめました。すぐに明らかになったのは、コメントが役に立つ理由は実にさまざまであり、機能や文体、長さ、更新頻度も大きく異なるということです。最終的にこの作業を分類という形にまとめました。

調査の過程で、私は9種類のコメントを特定しました。

  • 関数コメント
  • 設計コメント
  • 理由コメント
  • 教師コメント
  • チェックリストコメント
  • ガイドコメント
  • 自明なコメント
  • 負債コメント
  • バックアップコメント

私の考えでは、最初の6つはおおむね非常に前向きなコメントの形であり、最後の3つはやや疑問の余地があります。以降の各セクションでは、それぞれのタイプをRedisのソースコードからの例とともに分析していきます。

関数コメント

関数コメントの目的は、そもそも読者がコードを読まずに済むようにすることです。コメントを読んだ後は、コードを一定の規則に従うブラックボックスとして扱えるようにすべきです。通常、関数コメントは関数定義の冒頭に置かれますが、クラスやマクロ、あるいはインターフェースを定義するその他の機能的に独立したコードブロックを説明するために、別の場所に置かれることもあります。

rax.c:

    /* Seek the grestest key in the subtree at the current node. Return 0 on
     * out of memory, otherwise 1. This is an helper function for different
     * iteration functions below. */
    int raxSeekGreatest(raxIterator *it) {
    ...

関数コメントは、実際にはインラインのAPIドキュメントの一形態です。関数コメントが十分によく書かれていれば、利用者は多くの場合、関数やクラス、マクロなどの実装を読むことなく、元々読んでいた箇所(そのAPIを呼び出しているコード)へと戻ることができるはずです。

あらゆる種類のコメントの中でも、これらはプログラミングコミュニティ全体で必要だと最も広く受け入れられているものです。検討すべき唯一の点は、大半がAPIリファレンスであるようなコメントをコード自体の中に置くのが良い考えかどうかということです。私にとって答えはシンプルです。APIドキュメントにはコードと完全に一致していてほしいのです。コードが変更されれば、ドキュメントも変更されるべきです。そのため、関数などの冒頭に関数コメントを置くことで、APIドキュメントをコードのすぐそばに置くことができ、次の3つの成果が得られます。

  • コードが変更された際に、APIリファレンスを古いままにしてしまうリスクなく、同時にドキュメントも簡単に変更できます。
  • このアプローチにより、変更内容を最もよく理解しているはずの変更の作者自身が、APIドキュメントの変更も担当する可能性が最大化されます。
  • コードを読んでいるときに、関数やメソッドのドキュメントをその定義箇所ですぐに見つけられるため、コードとドキュメントの間でコンテキストスイッチをすることなく、コードだけに集中できます。

設計コメント

「関数コメント」が通常関数の冒頭に置かれるのに対し、設計コメントはより頻繁にファイルの冒頭に置かれます。設計コメントは、あるコード片がどのようなアルゴリズムや手法、テクニック、実装を用い、なぜそれらを用いているのかを基本的に述べます。コードで実装されている内容の、より高レベルな概観です。こうした背景があれば、コードを読むのはより簡単になります。さらに私は、設計メモが見つかるコードの方がより信頼できると感じる傾向があります。少なくとも開発過程のどこかの時点で、明示的な設計フェーズがあったことが分かるからです。

私の経験では、実装が提案する解決策が一見あまりにも単純に見える場合に、どのような競合する解決策があり、なぜ非常にシンプルな解決策で十分だと判断されたのかを述べるためにも、設計コメントはとても有用です。設計が正しければ、読者はその解決策が適切であり、その単純さが手抜きや基礎的なことしか書けないことから来たのではなく、プロセスを経た結果であることを納得できるでしょう。

bio.c:
     * DESIGN
     * ------
     *
     * The design is trivial, we have a structure representing a job to perform
     * and a different thread and job queue for every job type.
     * Every thread waits for new jobs in its queue, and process every job
     * sequentially.
     ...

理由コメント

理由コメントは、コードが何をしているかが極めて明確な場合であっても、なぜコードがあることを行っているのかという理由を説明します。Redisのレプリケーションコードからの次の例をご覧ください。

replication.c:

    if (idle > server.repl_backlog_time_limit) {
	/* When we free the backlog, we always use a new
	 * replication ID and clear the ID2. This is needed
	 * because when there is no backlog, the master_repl_offset
	 * is not updated, but we would still retain our replication
	 * ID, leading to the following problem:
	 *
	 * 1. We are a master instance.
	 * 2. Our replica is promoted to master. It's repl-id-2 will
	 *    be the same as our repl-id.
	 * 3. We, yet as master, receive some updates, that will not
	 *    increment the master_repl_offset.
	 * 4. Later we are turned into a replica, connect to the new
	 *    master that will accept our PSYNC request by second
	 *    replication ID, but there will be data inconsistency
	 *    because we received writes. */
	changeReplicationId();
	clearReplicationId2();
	freeReplicationBacklog();
	serverLog(LL_NOTICE,
	    "Replication backlog freed after %d seconds "
	    "without connected replicas.",
	    (int) server.repl_backlog_time_limit);
    }

関数呼び出しだけを見れば、疑問に思うことはほとんどありません。タイムアウトに達したら、メインのレプリケーションIDを変更し、セカンダリIDをクリアし、最後にレプリケーションバックログを解放します。しかし、バックログを解放する際に、なぜレプリケーションIDを変更する必要があるのかは、正確には明らかではありません。

これは、ソフトウェアがある程度の複雑さに達すると、絶えず起こる類のことです。関与するコードが何であれ、レプリケーションプロトコル自体がある程度の複雑さを持っているため、他の悪いことが起こらないようにするために、ある種のことを行う必要があります。おそらくこの種のコメントは、ある意味でシステムについて考察し、そうした複雑さがもはや不要になるように改善すべきかどうかを検討する機会でもあります。そうなればコメント自体も削除できます。しかし、何かを単純にすることが、しばしば別の何かをより困難にしたり、単に実現不可能だったり、後方互換性を壊すような将来の作業を必要としたりすることもあります。

もう一つ例を挙げます。

replication.c:

    /* SYNC can't be issued when the server has pending data to send to
     * the client about already issued commands. We need a fresh reply
     * buffer registering the differences between the BGSAVE and the current
     * dataset, so that we can copy to other replicas if needed. */
    if (clientHasPendingReplies(c)) {
        addReplyError(c,"SYNC and PSYNC are invalid with pending output");
        return;
    }

クライアントに送信すべき保留中の出力(過去のコマンドによるもの)がまだある状態でSYNCを実行すると、このコマンドは失敗すべきです。レプリケーションハンドシェイクの間、クライアントの出力バッファは変更を蓄積するために使われ、最初のレプリカとのフル同期のためにRDBファイルを作成している最中に接続してきた他のレプリカに対応するため、後で複製される可能性があるからです。これが私たちがそうする理由です。何をしているかは自明です。保留中の返信があるか?エラーを返します。なぜそうするのかは、コメントがなければかなり分かりにくいのです。

こうしたコメントは、レプリケーションのような複雑なプロトコルや相互作用を説明するときにだけ必要なのでは、と思うかもしれません。果たしてそうでしょうか。ファイルを変え、目的をまったく変えてみても、やはりそうしたコメントは至るところに見られます。

expire.c:

    for (j = 0; j < dbs_per_call && timelimit_exit == 0; j++) {
        int expired;
        redisDb *db = server.db+(current_db % server.dbnum);

        /* Increment the DB now so we are sure if we run out of time
         * in the current DB we'll restart from the next. This allows to
         * distribute the time evenly across DBs. */
        current_db++;
        ...

これは興味深い例です。時間が許す限り、異なるDBからキーを期限切れにしたいと考えています。ところが、現在のデータベースを処理するループの最後で次に処理する「データベースID」をインクリメントするのではなく、異なるやり方をしています。db変数で現在のDBを選択した後、すぐに次回この関数が呼ばれたときに処理すべき次のデータベースのIDをインクリメントするのです。こうすることで、1回の呼び出しで処理に時間をかけすぎたためにこの関数が終了した場合でも、同じデータベースから再開してしまい、同じデータベースの処理に集中するあまり、他のデータベースで論理的に期限切れとなるべきキーが溜まっていく、という問題を避けられます。

このようなコメントによって、なぜそのタイミングでインクリメントするのかを説明するとともに、次にコードを修正する人がその特性を維持すべきことを伝えています。コメントがなければ、このコードはまったく無害に見えることに注意してください。選択して、インクリメントして、作業を行う。インクリメントをより自然に見えるループの末尾に移さない明確な理由は何もないように見えるのです。

余談ですが、ループのインクリメントは元々のコードでは実際に末尾にありました。ある修正の際にそこへ移動され、同時にこのコメントが追加されたのです。ですから、これは一種の「リグレッションコメント」と言えるでしょう。

教師コメント

教師コメントは、コード自体や注意すべき副作用を説明しようとするものではありません。代わりに、コードが動作しているドメイン(例えば数学、コンピュータグラフィックス、ネットワーキング、統計、複雑なデータ構造など)について教えるものです。それは読者のスキルセットの範囲外かもしれませんし、単に詳細が多すぎてすべてを記憶から思い出すことができないものかもしれません。

バージョン5のLOLWUTコマンドは、画面に回転した正方形を表示する必要があります(http://antirez.com/news/123)。そのために基本的な三角法が使われています。使われている数学自体は単純ですが、Redisのソースコードを読むプログラマーの多くは数学的な背景を持っていないかもしれないため、関数の冒頭にあるコメントで、その関数の中で何が起こるのかが説明されています。

lolwut5.c:

    /* Draw a square centered at the specified x,y coordinates, with the specified
     * rotation angle and size. In order to write a rotated square, we use the
     * trivial fact that the parametric equation:
     *
     *  x = sin(k)
     *  y = cos(k)
     *
     * Describes a circle for values going from 0 to 2*PI. So basically if we start
     * at 45 degrees, that is k = PI/4, with the first point, and then we find
     * the other three points incrementing K by PI/2 (90 degrees), we'll have the
     * points of the square. In order to rotate the square, we just start with
     * k = PI/4 + rotation_angle, and we are done.
     *
     * Of course the vanilla equations above will describe the square inside a
     * circle of radius 1, so in order to draw larger squares we'll have to
     * multiply the obtained coordinates, and then translate them. However this
     * is much simpler than implementing the abstract concept of 2D shape and then
     * performing the rotation/translation transformation, so for LOLWUT it's
     * a good approach. */

このコメントには、関数自体のコードやその副作用、関数に関連する技術的な詳細に関することは何も含まれていません。説明は、ある目標を達成するために関数内部で用いられる数学的な概念にのみ限定されています。

教師コメントには非常に大きな価値があると考えています。読者がそうした概念を知らない場合に何かを教えたり、少なくともさらなる調査のための出発点を提供したりします。しかしこれは裏を返せば、教師コメントによってあるコードパスを読めるプログラマーの数が増えるということです。多くのプログラマーに読めるコードを書くことは、私の主要な目標の一つです。数学のスキルはなくても、非常に優秀なプログラマーとして素晴らしい修正や最適化に貢献できる開発者はいます。そして一般的に、コードは実行されるだけでなく読まれるべきものです。なぜならコードは人間が人間のために書くものだからです。

まともなコードを書くために、教師コメントがほとんど不可欠になる場合もあります。良い例がRedisの基数木(radix tree)の実装です。基数木は構造が複雑なデータ構造です。Redisの実装では、実装を進めながらデータ構造の理論全体を改めて記述し、さまざまなケースや、ノードをマージしたり分割したりする際にアルゴリズムが何を行うかを示しています。各コメントのセクションの直後には、直前に書かれた内容を実装するコードが続きます。基数木を実装したファイルに数か月触れていなかった後でも、私はそれを開いて数分でバグを修正し、また別の作業を続けることができました。基数木がどのように動作するかを改めて勉強し直す必要はありません。説明とコード自体が一体となって混ざり合っているからです。

コメントが非常に長いため、ここでは一部を抜粋して紹介します。

rax.c:

    /* If the node we stopped at is a compressed node, we need to
     * split it before to continue.
     *
     * Splitting a compressed node have a few possible cases.
     * Imagine that the node 'h' we are currently at is a compressed
     * node contaning the string "ANNIBALE" (it means that it represents
     * nodes A -> N -> N -> I -> B -> A -> L -> E with the only child
     * pointer of this node pointing at the 'E' node, because remember that
     * we have characters at the edges of the graph, not inside the nodes
     * themselves.
     *
     * In order to show a real case imagine our node to also point to
     * another compressed node, that finally points at the node without
     * children, representing 'O':
     *
     *     "ANNIBALE" -> "SCO" -> []

     ... snip ...

     * 3a. IF $SPLITPOS == 0:
     *     Replace the old node with the split node, by copying the auxiliary
     *     data if any. Fix parent's reference. Free old node eventually
     *     (we still need its data for the next steps of the algorithm).
     *
     * 3b. IF $SPLITPOS != 0:
     *     Trim the compressed node (reallocating it as well) in order to
     *     contain $splitpos characters. Change chilid pointer in order to link
     *     to the split node. If new compressed node len is just 1, set
     *     iscompr to 0 (layout is the same). Fix parent's reference.

     ... snip ...

        if (j == 0) {
            /* 3a: Replace the old node with the split node. */
            if (h->iskey) {
                void *ndata = raxGetData(h);
                raxSetData(splitnode,ndata);
            }
            memcpy(parentlink,&splitnode,sizeof(splitnode));
        } else {
            /* 3b: Trim the compressed node. */
            trimmed->size = j;
            memcpy(trimmed->data,h->data,j);
            trimmed->iscompr = j > 1 ? 1 : 0;
            trimmed->iskey = h->iskey;
            trimmed->isnull = h->isnull;
            if (h->iskey && !h->isnull) {
                void *ndata = raxGetData(h);
                raxSetData(trimmed,ndata);
            }
            raxNode **cp = raxNodeLastChildPtr(trimmed);
        ...

ご覧のとおり、コメント内の説明はコード内で同じラベルと対応しています。この形式ですべてをお見せするのは難しいので、全体像を把握したい場合は、次のファイル全体をご確認ください。

https://github.com/antirez/redis/blob/unstable/src/rax.c

このレベルのコメントは、すべてのものに必要なわけではありません。しかし基数木のようなものは、本当に細かな詳細やコーナーケースに満ちています。それらは思い出すのが難しく、特定の詳細は特定の実装に固有のものです。もちろん、これを連結リストに対して行ってもあまり意味はありません。やる価値があるかどうかを見極めるのは、個人の感性にかかっています。

チェックリストコメント

これは非常によく見られる、ちょっと奇妙なタイプです。言語の制限や設計上の問題、あるいは単にシステムに自然に生じる複雑さのために、ある概念やインターフェースを一か所に集約することができず、コードの中に「別の場所で何かをすることを忘れないように」と教えてくれる箇所があるのです。一般的な概念は次のようなものです。

    /* Warning: if you add a type ID here, make sure to modify the
     * function getTypeNameByID() as well. */

理想的な世界では、これは決して必要ないはずですが、実際には逃れられないこともあります。例えばRedisの型は「オブジェクト型」構造を使って表現することもでき、すべてのオブジェクトが自分が属する型へリンクするようにすれば、次のように書けるでしょう。

    printf("Type is %s\n", myobject->type->name);

しかし実際はどうでしょう。私たちにとってはコストが高すぎるのです。なぜならRedisのオブジェクトは次のように表現されているからです。

    typedef struct redisObject {
        unsigned type:4;
        unsigned encoding:4;
        unsigned lru:LRU_BITS; /* LRU time (relative to global lru_clock) or
                                * LFU data (least significant 8 bits frequency
                                * and most significant 16 bits access time). */
        int refcount;
        void *ptr;
    } robj;

私たちは型を表すのに64ビットではなく4ビットを使っています。これは、物事が本来あるべきほど集約的かつ自然ではないことがある理由を示す一例にすぎません。状況がそうであるとき、役に立つことがあるのが防御的なコメントです。あるコードセクションに手を加えた際に、コードの他の部分も修正する必要があることを思い出させてくれるのです。具体的には、チェックリストコメントは次のいずれか、あるいは両方を行います。

  • 何かが修正された際に実行すべき一連のアクションを伝えます。
  • 特定の変更をどのように行うべきかについて警告します。

blocked.cにある、新しいブロッキングタイプが導入された際の別の例です。

blocked.c:

     * When implementing a new type of blocking opeation, the implementation
     * should modify unblockClient() and replyToBlockedClientTimedOut() in order
     * to handle the btype-specific behavior of this two functions.
     * If the blocking operation waits for certain keys to change state, the
     * clusterRedirectBlockedClientIfNeeded() function should also be updated.

チェックリストコメントは、特定の「理由コメント」が使われる文脈と似た状況でも有用です。なぜあるコードがある場所で、何かの前や後に実行されなければならないのかが自明でない場合です。しかし、理由コメントがなぜその文がそこにあるのかを教えてくれるのに対し、同じケースで使われるチェックリストコメントは、コードの挙動を壊さずに修正したい場合に従うべきルール(この場合は、所定の順序を守ること)を伝えることに、より重点が置かれています。

cluster.c:

    /* Update our info about served slots.
     *
     * Note: this MUST happen after we update the master/replica state
     * so that CLUSTER_NODE_MASTER flag will be set. */

チェックリストコメントは、特定の操作の順序が極めて重要となるLinuxカーネルの内部で非常によく見られます。

ガイドコメント

私はガイドコメントを濫用していると言えるほど多用しており、おそらくRedisのコメントの大半はガイドコメントです。しかもガイドコメントは、多くの人がまったく無用だと考えているコメントそのものです。

  • コードから明らかでないことを述べているわけではありません。
  • ガイドコメントの中に設計上のヒントはありません。

ガイドコメントがすることは一つだけです。読者を手取り足取り導き、明確な区切りやリズムを与え、これから読む内容を紹介することで、ソースコードに書かれている内容を処理する間、読者をサポートするのです。

ガイドコメントが存在する唯一の理由は、コードを読むプログラマーの認知負荷を下げることです。

rax.c:

    /* Call the node callback if any, and replace the node pointer
     * if the callback returns true. */
    if (it->node_cb && it->node_cb(&it->node))
	memcpy(cp,&it->node,sizeof(it->node));

    /* For "next" step, stop every time we find a key along the
     * way, since the key is lexicographically smaller compared to
     * what follows in the sub-children. */
    if (it->node->iskey) {
	it->data = raxGetData(it->node);

	return 1;
    }

上記のコードに対して、コメントが何かを付け加えているわけではありません。上記のガイドコメントはコードを読むのを助け、さらに自分が正しく理解できていることを確認させてくれます。さらに例を挙げます。

networking.c:

    /* Log link disconnection with replica */
    if ((c->flags & CLIENT_SLAVE) && !(c->flags & CLIENT_MONITOR)) {
        serverLog(LL_WARNING,"Connection with replica %s lost.",
            replicationGetSlaveName(c));
    }

    /* Free the query buffer */
    sdsfree(c->querybuf);
    sdsfree(c->pending_querybuf);
    c->querybuf = NULL;

    /* Deallocate structures used to block on blocking ops. */
    if (c->flags & CLIENT_BLOCKED) unblockClient(c);
    dictRelease(c->bpop.keys);

    /* UNWATCH all the keys */
    unwatchAllKeys(c);
    listRelease(c->watched_keys);

    /* Unsubscribe from all the pubsub channels */
    pubsubUnsubscribeAllChannels(c,0);
    pubsubUnsubscribeAllPatterns(c,0);
    dictRelease(c->pubsub_channels);
    listRelease(c->pubsub_patterns);

    /* Free data structures. */
    listRelease(c->reply);
    freeClientArgv(c);

    /* Unlink the client: this will close the socket, remove the I/O
     * handlers, and remove references of the client from different
     * places where active clients may be referenced. */
    unlinkClient(c);

Redisは文字通りガイドコメントだらけで、基本的にどのファイルを開いても大量に見つかります。なぜわざわざそうするのでしょうか。このブログ記事でこれまで分析してきたすべてのコメントタイプの中で、これが最も主観的なものであることは認めます。こうしたコメントがないコードを劣っていると評価するわけではありませんが、人々がRedisのコードを読みやすいと見なすのであれば、その理由の一端は間違いなくガイドコメントにあると固く信じています。

ガイドコメントには、述べたもの以外にも有用性があります。コードを独立したセクションにはっきりと区切ってくれるため、コードへの追加がランダムな場所ではなく、適切なセクションに挿入される可能性が非常に高くなります。関連する文が近くにまとまっていることは、可読性における大きな勝利です。

また、unlinkClient()関数が呼び出される直前のガイドコメントにも注目してください。このガイドコメントは、関数が何をしようとしているかを簡潔に読者に伝え、全体像だけを知りたい場合に、わざわざ関数の中までジャンプして読む必要をなくしてくれます。

自明なコメント

ガイドコメントは非常に主観的な道具です。好きか嫌いかは人それぞれでしょう。私は大好きです。しかし、ガイドコメントは非常に悪いコメントへと堕落する可能性があります。容易に「自明なコメント」になってしまうのです。自明なコメントとは、コメントを読む認知負荷が、関連するコードを読む負荷と同じかそれ以上であるようなガイドコメントのことです。次のような形の自明なコメントは、多くの書籍が避けるべきだと述べているものとまさに同じです。

    array_len++;	/* Increment the length of our array. */

ですからガイドコメントを書くのであれば、自明なものにならないように気をつけてください。

負債コメント

負債コメントは、ソースコード自体の中にハードコードされた技術的負債の表明です。

t_stream.c:

    /* Here we should perform garbage collection in case at this point
     * there are too many entries deleted inside the listpack. */
    entries -= to_delete;
    marked_deleted += to_delete;
    if (entries + marked_deleted > 10 && marked_deleted > entries/2) {
	/* TODO: perform a garbage collection. */
    }

上記のスニペットは、Redisストリームの実装から抜粋したものです。Redisストリームでは、XDELコマンドを使って中間から要素を削除できます。これはさまざまな場面で有用となり得ます。特に、どのようなデータ構造やシステムを使っていても特定のデータを保持してはならないという、プライバシー規制の文脈で有用です。これは基本的に末尾追加型のデータ構造にとっては非常に奇妙なユースケースですが、ユーザーが中間のアイテムの50%以上を削除し始めると、ストリームは「マクロノード」から構成されることで断片化し始めます。エントリは単に削除済みとしてフラグが立てられるだけで、実際に回収されるのは、あるマクロノード内のすべてのエントリが解放されたときのみです。したがって大量の削除は、ストリームのメモリ挙動を変えてしまいます。

今のところ、ストリーム内のほとんどの履歴をユーザーが削除するとは想定していないため、これは問題にならないように見えます。しかし将来的にはガベージコレクションを導入したくなる可能性もあります。削除されたエントリと既存のエントリの比率がある水準に達したときに、マクロノードを圧縮するのです。さらにガベージコレクションの後、近くのノード同士を結合することも考えられます。後になってガベージコレクションを行うためのエントリーポイントがどこだったかを思い出せなくなるのが少し不安だったので、TODOコメントを置き、トリガー条件まで書き記しました。

これはおそらく良いやり方ではありません。より良い考えは、ファイル冒頭の設計コメントの中に、なぜ現在GCを行っていないのか、そして後で追加したい場合のGCのエントリーポイントはどこなのかを書いておくことでした。

FIXME、TODO、XXX、「This is a hack」などは、すべて負債コメントの形です。一般的にはあまり良いものではなく、私も避けるようにしていますが、常に避けられるわけではありません。そして問題を永遠に忘れてしまうよりは、ソースコードの中にメモを残しておく方を好むこともあります。少なくとも、そうしたコメントを定期的にgrepして、メモをより良い場所に移せないか、あるいは問題がもはや関係なくなったか、すぐに修正できないかを検討すべきです。

バックアップコメント

最後に、バックアップコメントとは、開発者が新しいコードへの変更に自信が持てないために、あるコードブロックや関数全体の古いバージョンをコメントアウトして残すものです。不可解なのは、Gitがある現在でもこれが起こることです。何年も前のコミットの中に、より健全あるいは安定していると考えられるコード断片を失ってしまうことへの不安感があるのだろうと思います。

しかしソースコードはバックアップを取るための場所ではありません。関数やコードの一部について古いバージョンを保存したいのであれば、あなたの作業はまだ完了しておらず、コミットすべきではありません。新しい関数が過去のものより優れていることを確かめるか、確信が持てるまで自分の開発ツリーの中だけに留めておくべきです。

これで私の分類は終わりです。結論をまとめてみましょう。

分析ツールとしてのコメント

コメントは、ステロイドで強化されたラバーダック・デバッグのようなものです。ただし話しかける相手はラバーダックではなく、コードの将来の読者です。将来の読者はラバーダックよりも手強く、Twitterを使うこともできます。その過程で、あなたは自分が述べていることが許容できるか、恥ずかしくないか、十分に良いかを本気で理解しようとします。そしてもしそうでなければ、宿題をやり直し、よりまともなものを作り上げるのです。

これはドキュメントを書くときに起こるのと同じプロセスです。書き手は、あるコード片が何をするのか、どのような保証があり、どのような副作用があるのかという要点を伝えようとします。これはしばしばバグ発見の機会となります。何かを説明している最中に、それに穴があることに気づくのは非常に簡単です。ある挙動について確信が持てないために、すべてをうまく説明できないのです。その挙動は、複雑さの中からランダムに浮かび上がってきたものに過ぎません。そんなことは本当に避けたいので、戻ってすべてを修正するのです。私はこれこそ、コメントを書く素晴らしい理由だと考えています。

良いコメントを書くことは、良いコードを書くことよりも難しい

コメントを書くことは、あまり高尚でない仕事だと思うかもしれません。なにしろあなたはコードが書けるのですから。しかしこう考えてみてください。コードとは、文や関数呼び出しの集合であり、あるいはあなたのプログラミングパラダイムが何であれその集合です。正直なところ、コードが良くなければ、そうした文があまり意味をなさないこともあります。コメントは常に何らかの設計プロセスが進行していることを要求し、書いているコードをより深い意味で理解することを求めます。さらに良いコメントを書くためには、文章を書くスキルを磨かなければなりません。その同じ文章力は、メールやドキュメント、設計書、ブログ記事、コミットメッセージを書く際にも役立つのです。

私がコードを書くのは、何よりも共有し、伝えたいという切迫した思いがあるからです。コメントはコードを補い、助け、私たちの努力を描写してくれます。そして結局のところ、私はコード自体を書くのと同じくらい、コメントを書くことが大好きなのです。

(このブログ記事の執筆中にフィードバックをくれたMichel Martens氏に感謝します)

原文は Salvatore Sanfilippo により に公開されました。

この記事は「muse-spark-1.2-contributor」を使用して翻訳されました。