CleanCode Chapter 4:Comments

Clean Code: A Handbook of Agile Software Craftsmanship (Robert C. Martin Series)

Clean Code: A Handbook of Agile Software Craftsmanship (Robert C. Martin Series)

この前のエントリに引き続き、CleanCode から今日は Chapter 4:Comments の感想というか、概要 (概要?)。
コメントだけで一章使って説明してる本は知る限りではこの本のみ (のはず)。

いいコメント

有益なコメントだけじゃなくて、必要なコメントも「いいコメント (Good Comments)」というくくりにしている。
いいコメントの一覧。

  • 著作権表示とかライセンスとか作者などといった、Leagal Comment
  • 情報を与えるコメント (Informative Comment)
  • 意図の説明
  • 説明 (Clarification)
  • 重大な警告
  • TODO コメント
  • 増幅 (Amplification)
  • public な API のドキュメンテーションコメント
著作権表示とかライセンスとか作者などといった、Legal Comment

でも可能なら外に出せ。

情報を与えるコメント (Informative Comment)
// Returns an instance of the Responder begin tested.
protected abstract Responder responderInstance();
CleanCode

でも名前を変更すれば必要なくなることも多い。上の例では、responderBeingTested に変更するとか。

意図の説明

「なぜそうしたか」や「なぜそうしないか」についてのコメントのこと。

説明 (Clarification)

これは例を見てもらうのが一番分かりやすい。

public void testCompareTo() throws Exception
{
  WikiPagePath a = PathParser.parse("PageA");
  WikiPagePath ab = PathParser.parse("PageA.PageB");
  WikiPagePath b = PathParser.parse("PageB");
  WikiPagePath aa = PathParser.parse("PathA.PathA");
  WikiPagePath bb = PathParser.parse("PathB.PathB");
  WikiPagePath ba = PathParser.parse("PathB.PathA");
  
  assertTrue(a.compareTo(a) == 0);    // a == a
  assertTrue(a.compareTo(b) != 0);    // a != b
  assertTrue(ab.compareTo(ab) == 0);  // ab == ab
  assertTrue(a.compareTo(b) == -1);   // a < b
  assertTrue(aa.compareTo(ab) == -1); // aa < ab
  assertTrue(ba.compareTo(bb) == -1); // ba < bb
  assertTrue(b.compareTo(a) == 1);    // b > a;
  assertTrue(ab.compareTo(aa) == 1);  // ab > aa
  assertTrue(bb.compareTo(ba) == 1);  // bb > ba
}
CleanCode

こんなコメント。ただ、compareTo に関しては、常に 0 との大小比較を行えばそれほどコメントが必要だとは思えないなぁ。

a.compareTo(b) == 0;    // a == b の意味
a.compareTo(b) <  0;    // a <  b の意味
a.compareTo(b) >  0;    // a >  b の意味
a.compareTo(b) <= 0;    // a <= b の意味
a.compareTo(b) >= 0;    // a >= b の意味

こんな感じに、不等号の向きと a と b の順番とが一致する。


それと、本ではこの種のコメントのリスクについても言及している。
どういうことかというと、この種のコメントが間違っていた場合、それを正すのが非常に困難ということ。
例えば、引用したコメントがどこか間違っていた場合を考えてみるといい。


ご利用は計画的に。

重大な警告

これも例を見るのが分かりやすい。

// Don't run unless you
// have some time to kill.
public void _testWithReallyBigFile()
{
  writeLinesToFile(10000000);
  
  response.setBody(testTile);
  response.readyToSend(this);
  String responseString = output.toString();
  assertSubString("Content-Length: 10000000", responseString);
  assertTrue(bytesSent > 10000000);
}
CleanCode

一応説明しておくと、JUnit3 では、test からはじまるメソッドを実行するようになっていて、上の例ではアンダーバーが付いているからテストが無効化されている。で、なんで無効化されているかというのがコメントに書いてある。
ただ、本の中にあるように、JUnit4 ではアノテーションを使って、

@Test
@Ignore("Takes too long to run")
public void testWithReallyBigFile() {
    ...
}

とするように変更された。
コメントじゃなくてアノテーションを使うことで、プログラム側から弄ることができるので、こういう用途のコメントはどんどんアノテーションに置き換えればいいと思う。

TODO コメント

TODO コメントは説明する必要はないと思う。
一般的な IDE だったら TODO コメントをリストアップしてくれたり、強調表示してくれたりするので活用すべき。乱用はいけないけどね*1

増幅 (Amplification)

重大な警告の弱い版?意図の説明とも取れるような・・・よくわからん。

public な API のドキュメンテーションコメント

他のコメントと同じように、ドキュメンテーションコメントも誤解・nonlocal・間違える可能性はあるので、注意しろよ、と。

悪いコメント

ほとんどのコメントがこのカテゴリに属するそうな。
悪いコメントの一覧。

  • 何言ってるか分からないコメント
  • 冗長なコメント (Redundant Comments)
  • 誤解を招くコメント
  • 強制されたコメント
  • 履歴 (Journal Comments)
  • ノイズ (Noise Comments)
  • ぞっとするようなノイズ (Scary Noise)
  • 関数や変数を使えばいいようなコメント
  • マーカー
  • 閉じ括弧のコメント
  • 属性や署名
  • コメントアウト
  • HTML コメント
  • ふさわしくない場所に記述されたコメント
  • 饒舌すぎるコメント
  • 説明の足りないコメント
  • 関数のヘッダ
  • public ではないコードのドキュメンテーションコメント
  • 例示

ノイズとぞっとするようなノイズがどう違うかわからい。

冗長なコメント (Redundant Comments)

例えば、何をやっているか説明するようなコメント。
本では、

// Utility method that returns when this.closed is true. Throws an excetion
// if the timeout is reached.
public synchronized void waitForClose(final long timeoutMillis)
throws Exception
{
  if (!closed)
  {
    wait(timeoutMillis);
    if (!closed)
      throw new Exception("MockResponseSender could not be closed");
  }
}
CleanCode

こんな例の他に、冗長で役に立たないドキュメンテーションコメントについても Tomcat のコードを例に挙げて言及している。
ただ、その部分は他とかぶるので後で。

強制されたコメント

コーディング規約とかで「必ずドキュメンテーションコメントを記述すること」とかあると、

/**
 * @param workCode 作業コード
 * @param worker 作業者
 */
public void startWork(String workCode, Worker worker) {
    ...
}

みたいな何の役にも立たないようなコメントが書かれたりするという話。
役に立たないだけならまだしも、こんなものが大量にあると、コードが埋もれてしまって読みにくくなる。

履歴 (Journal Comments)、属性や署名

これを強制されずに書いてるプログラマが一体どれほどいるんだろう。当然、バージョン管理システムを使ったほうがいい。
ちなみに、履歴 (Journal Comments) はファイルの先頭に書かれるようなコメント、属性や署名ってのは、「ここからここまではだれだれが追加した」とか、そういった類のコメントのこと。

ノイズ (Noise Comments)
/**
 * Default constructor.
 */
protected AnnualDateRule() {
}
/** The day of the month. */
    private int dayOfMonth;
/**
 * Returns the day of the month.
 *
 * @return the day of the month.
 */
public int getDayOfMonth() {
  return dayOfMonth;
}
CleanCode

こんなドキュメンテーションコメント、書いた覚えありませんか?

ぞっとするようなノイズ (Scary Noise)

上との違いは良くわからない。

/** The name. */
private String name;

/** The version. */
private String version;

/** The licenceName. */
private String licenceName;

/** The version. */
private String info;
CleanCode

ドキュメンテーションコメントは簡単にノイズになるという話。info のコメントはコピペしてそのまま修正されずに残っていて、こういうコメントが「Scary Noise」であると言っているのかな?
とにかく、上と合わせて考えると、「名前見て明らかなものにドキュメンテーションコメントを書いても、ノイズにしかならない」ということだと思う。
まぁこれは実際その通りで、例えば、

  • final な getter/setter などの単純なメソッド
  • public/protected 以外のメンバ
  • 非 public なクラスの public なメソッド*2

なんかはドキュメンテーションコメントは不要かも。

閉じ括弧のコメント

これについてはClean Code vs. プログラミングのセオリー - ぐるぐる〜で書いたけど、コメントの「Chapter 3: Function に書かれた内容」は違ってたw
でも、

try to shorten your function instead.
適当訳:かわりに関数を短くするように努力すべき。

CleanCode

って言ってるから、やっぱり関数内の話なのは間違いないけど。

HTML コメント

確かに、タグでごちゃごちゃしてる上、実体参照やら何やらを使われだすと、直接読むのはつらいよなー。
出力が HTML だからと言って、入力まで HTML である必要はない。読みにくい上書きにくいなんて、いいところないわけだし。

関数のヘッダ

短い関数にはコメントなんて不要だ、という話。まぁ確かに、短い関数に数行のコメント書くのは非生産的だよなぁ、と思う。
短い関数には、それに応じた分かりやすい名前さえ付いていればコメントは不要。

Clean Code: A Handbook of Agile Software Craftsmanship (Robert C. Martin Series)

Clean Code: A Handbook of Agile Software Craftsmanship (Robert C. Martin Series)

*1:BTS/ITS に記載すべきものまで TODO コメントを使ってしまう、ってのはありがちかも

*2:インターフェイスを実装してたりすると public にせざるを得ないけど、クラス自体は外から見えないとかはよくある話