Qiita Teams that are logged in
You are not logged in to any team

Log in to Qiita Team
Community
OrganizationEventAdvent CalendarQiitadon (β)
Service
Qiita JobsQiita ZineQiita Blog
3
Help us understand the problem. What are the problem?

More than 1 year has passed since last update.

コメントは書くな(Stop Writing Code Comments)の翻訳

はじめに

コーディングをするときは、コメントをたくさん書け。
そう言われて育ちました。
しかし、この記事を見て、衝撃を受けました。
簡単に言うと、「できるだけコメントがなくてもわかりやすいコードを書くようにしよう」という記事です。
その記事の最後に「Stop Writing Code Comments」という記事の紹介がされており、コードのコメントについてかなりわかりやすく書かれていたので、自分用に翻訳・要約しておこうと思いました。
以下では、ちょいちょい要約しながら訳していきます。

コメントを書くな

コメントが必要ないような、明確で表現豊かなコードを書くべきである。
変数名、関数名は見ただけで役割がわかるようなものにするべきである。
コメントを書く必要があるということは、きれいなコードを書けていないということと同じである。
コメントを書くたびに鋭い胃痛を感じなければならない。

コメントは失敗を覆い隠す

変数や関数の上によくコメントが書かれている。
これらのコメントのせいで、表現豊かな名前を付けられなくなったり、関数に2つ以上の役割を持たせてしまう。

「名付け」はコードを書く上で極めて重要だ。
他人が正確かつ的確にコードを読めるように、名付けに注力すべきである。

// find employees by status
List<Employee> find(Status status) {
  ...
}

上の例では、findという名前だけでは説明不足なため、この関数の役割をコメントで説明している。
もしこの関数findが他のプログラムで呼び出されたとしたら、何をするものなのか全くわからない。
何をfindするのか?どのようにfindするのか?

いちいち各関数の上に書いてあるコメントなんて読んでられない。

List<Employee> getEmployeesByStatus(Status status) {
  ...
}

これならば名前を読むだけでこの関数が何をするのかがわかる。
コメントなんていらない。

余計なコメント

余計なコメントは混乱を招くため、完全に不要だ。
余計なコメントをたくさん書くと、読者はそれを読み飛ばすようになってしまうため、重要なコメントですら読み飛ばされてしまう恐れがある。

// this function sends an email
void sendEmail() {
  ...
}

// this class holds data for an employee
public class Employee {
  ...
}

/**
 * @param title The title of the CD
 * @param author The author of the CD
 * @param tracks The number of tracks on the CD
 */
public void addCd(String title, String author, int tracks) {
  ...
}

最後のコメントは冗長な決まり文句である。
多くの組織では、すべての関数とクラスの上にこれを書かなければならない。
上司にこれを書くように言われたら、断ろう。

間違った抽象化

関数には次の規則がある。

  1. 関数は1つのことだけをすべきである。
  2. 関数は小さくあるべきである。

次の例を見てみよう。

// This function calculates prices, compares to sales
// promotions, checks if prices are valid, then 
// send an email of promotion to user
public void doSomeThings() {

  // Calculate prices
  ...
    ...
    ...

  // Compare calculated prices with sales promotions
  ...
    ...
    ...

  // Check if calculated prices are valid
  ...
    ...
    ...

  // Send promotions to users
  ...
    ...
    ...
}

もしもコードの各部分をそれぞれ関数にできるのならば、リファクタリングしよう。
上手くカプセル化できれば、読むだけで役割がわかるようになる。

さっきの例をリファクタリングすると、次のようになる。

public void sendPromotionEmailToUsers() {
  calculatePrices();
  compareCalculatedPricesWithSalesPromotions();
  checkIfCalculatedPricesAreValid();
  sendPromotionEmail();
}

まず、可読性が向上する。
関数名を見れば何をするものなのかがわかるし、詳細が知りたければ関数の中身を見れば良い。

次に、テストがしやすくなる。
上手くカプセル化ができていれば、機能ごとにテストができる。
2つ以上の機能を持つ関数はテストがしにくいものである。

最後に、リファクタリングがしやすくなる。
カプセル化したことにより、それぞれのロジックはそれぞれの関数に分かれている。もしも変更したい部分があったとしても、該当する関数だけを書き換えれば良い。

コメントアウトされたコード

コメントアウトされたコードを残してはいけない。

/*
public void oldFunction() {
  noOneRemembersWhyIAmHere();
  tryToUnCommentMe();
  iWillProbablyCauseABuildFailure();
  haHaHa();
}
*/

コメントアウトを外したとき、きちんとコンパイルできる保証はない。他の機能を無効にするかもしれない。ただちに消そう。
もしもそれが後で必要になるのならば、バージョン管理システムで確認しよう。

TODOコメント

TODOコメントを書いてはいけない。他の人が見たときに、そのTODOが完了したものなのかどうかを判断できない。ただちに消そう。
唯一TODOコメントを書いても良いのは、他のチームメイトとのマージをするときである。

コメントは嘘をつく

コードを変更して、他の場所にあるコメントを無効にした場合、それを知るすべがない。次の例を見てみよう。

public class User {
  ...

  // this holds the first and last name of the user 
  String name;

  ...
}

この後に、firstNameとlastNameに分けたとしよう。

public class User {
  ...

  // this holds the first and last name of the user 
  String firstName;
  String lastName;

  ...
}

こうなるとコメントは間違っている。コメントを書き換えなければならない。でもそれをいちいち手動で行うのは大変だ。
では次はどうだろうか。

// Process employees based on status
void processEmployees() {
  ...
  List<Employee> employees = findEmployees(statusList);
  ...
}

// this finds Employees by a list of statuses
List<Employee> findEmployees(List<String> statusList) {
  ...
}

関数findEmployeesをステータスリストの代わりに名前で検索するために変更したとしよう。

// Process employees based on status
void processEmployees() {
  ...
  List<Employee> employees = findEmployees(statusList);
  ...
}

// this finds Employees by a list of statuses
List<Employee> findEmployees(List<String> nameList) {
  ...
}

findEmployeesの上のコメントを変えなければいけない。ただ、それだけで良いだろうか?いいや違う。
processEmployeesの上も変えなければならない。
この小さいリファクタリングによって、あと何個のコメントを変更しなければならないだろうか?

解決策はこうだ。

void processEmployees() {
  ...
  List<Employee> employees = findEmployeesByName(nameList);
  ...
}

List<Employee> findEmployeesByName(List<Name> nameList) {
  ...
}

関数の名前を正確かつ的確に付ければ、コメントの変更を気にする必要はない。

コメントしても良いとき

コメントが必要なときもある。

複雑な表現
複雑なSQLや正規表現にはコメントをつけよう。これらは明確で表現豊かな名前を付けるのが難しい場合がある。これらのコードの上にコメントを付ければ、読む人の理解の助けになる。

// format matched kk:mm:ss EEE, MMM dd, yyy
Pattern timePattern = Pattern.compile("\\d*:\\d*:\\d* \\w*, \\w*, \\d*, \\d*");

警告
もしも、コードによって副作用や悪いことが起りえるのであれば、コメントを残しても大丈夫。そういうコメントは、コードの不可解なふるまいの警告として機能し、コードに価値を付加することがある。

意図の明確化
もしも表現豊かなコードを書くことができないのであれば、読み間違えに備えてコメント書こう。説明が不十分なコードを放置するのは良くない。

コメントを書く場合は、関数や変数の変すぐ上に書こう。警告はそのコードの上や横に書こう。離れた場所にコメントを書くと、コードを書き換えた場合に合わせてコメントを変更するのが難しくなる。

おわりに

以上、コメントに関して思うことを述べた。
確かにコメントが必要なときもある。
だが、是非とも多くのコメントを書くのをやめていただきたい。

Why not register and get more from Qiita?
  1. We will deliver articles that match you
    By following users and tags, you can catch up information on technical fields that you are interested in as a whole
  2. you can read useful information later efficiently
    By "stocking" the articles you like, you can search right away
3
Help us understand the problem. What are the problem?