読者です 読者をやめる 読者になる 読者になる

$shibayu36->blog;

株式会社はてなでエンジニアをしています。プログラミングや読書のことなどについて書いています。

最近コード中のTODOコメントの書き方を工夫している

tech

コード中に後でやろうと思って以下の様なTODOコメントを書くことがあります。TODOコメントというのは

# TODO: 後でリファクタリングしたい
...
# TODO: 投稿機能ができたら置き換えること
...

みたいなやつです。


コード中にTODOコメントを気軽に書いてしまいがちですが、よくTODOコメントが放置されて気づいたらプロジェクト中に大量のTODOコメントが書かれたりすることがあります。直せる量を超えてくると、直すモチベーションも下がってきて、結局ただのコメントと同じ状態になります。

最近いろいろ工夫して、TODOコメントの書き方を変えたところ、そこそこうまく回ったのでメモしておきます。

TODOコメントの問題点

問題点として次のものがあると考えました。

  • (1) 書く人によって形式がバラバラ
    • TODO, XXX, FIXMEなどバラバラだったりする
  • (2) TODOコメントの温度感が分からない
    • それは期限があるのか、必ず直さなければならないのか、直せるなら直したいのか、とりあえず書いておきたいだけなのか

これらの結果として、

  • 必ず直さなければならないTODOが埋もれてしまい、直す前にリリースなどしてしまい、バグを引き起こしてしまう
  • TODOコメントが多すぎて放置する

などの問題が起こっていそうです。

解決策

(1)の形式がバラバラ問題については統一すれば良いだけなので簡単です。テストなどで自動で検知しても良いですが、ひとまずコーディング規約でどれかに統一するように書きました。今回は「TODO」で統一するようにしました。

(2)のTODOコメントの温度感が分からない問題についてはいろいろ試してみましたが以下のようにしてみました。

  • 期限付きでMUSTでやらなければならないことのみにTODOというラベルをつける
    • 例) ある日にリリースがあり、それまでには終わらせたい
    • 例) 新機能をリリースするので、リリース前には終わらせたい
    • 例) 今はサービス内でイベントが行われていて、それが終わったら直したい
  • 期限がないもので、やっておきたいものとかにはTODOというラベルを付けず、温度感とかを含めたコメントのみで済ます
  • TODOに期限を示すラベルをつけ、期限が同じものについては同じラベルを使う
    • 例) 5/20にリリースし、それまでに直したいなら、TODO(5/21release): ...
    • 例) 検索機能をリリースするまでに直したいなら、TODO(search_release): ...
    • 例) イベントが終わったら直したいなら、TODO(spring_event_finished): ...


以上、(1)(2)の解決案により、コーディング規約は最終的に

  • TODOコメントは必ずやらなければならないことにのみ付ける
  • フォーマットは TODO(期限) とする
    • 例) TODO(5/21release), TODO(search_release)

のようになりました。


ラベルが統一されて、MUSTのみにTODOコメントが付けられて、期限が書かれていれば、TODOラベルが残っていたらまずいということになります。ただし形式が決まっているおかげで、以下のようにしてある期限のTODOを全部洗い出すのも簡単です。これでやるべきことをやる前にリリースしてしまってバグる、みたいなことも減ってありがたいですね。

# 全てのTODOを洗い出す
$ git grep 'TODO'
# 5/21のTODOを洗い出す
$ git grep -e 'TODO' --and -e '5/21release'
# 検索リリースのTODOを洗い出す
$ git grep -e 'TODO' --and -e 'search_release'

また以下の結果をCIで動かしておいて、適当にグラフ化しておくと、無駄にTODOが増えていないか確認もできてよいですね。

# TODOの数を出す
$ git grep 'TODO' | wc -l

まとめ

今回はTODOコメントを書くときの最近の工夫について書きました。他にもこういうことをしているとかあれば教えて下さい。

2016/05/09 8:00追記

id:takc923 必ずやらないといけない期限が決まってるTODOはチケット化してる
id:nntsugu あるリリースまでに解決しないといけないことは、チケットを切ってバックログに積み、Versionを付けておくなりして1箇所に集約したほうが安全かな。"@TODO チケットのURL"とすると現状も確認便利かも。

ブコメでの上の指摘は良い指摘と思ったので、補足を追加します。


まず僕も期限が決まっているTODOはgithubでissue化し、かつチェックリストを作ったりします。ただしそれはTODOコメントと粒度が異なります。


例を使って説明をします。僕は最近はPullRequestは出来るだけ小さくし、新機能を追加する場合もユーザーに見えないようにした上でどんどんmasterブランチにmergeしたりしています。見えない形というのは、「ある機能のページ」や「それへの導線」は開発中にしか見えないようにする、みたいな感じです。その場合、

  • 「本番でもページを見えるようにする」というTODOはgithubのissueにまとめておきたい
  • もっと細かい単位で、「本番でもページを見えるようにする」ために「どこ」を直すかについてはTODOコメントとしていろいろ書いておく
    • Controllerでアクセス制御をしている部分とか、導線を消している部分とか、ちゃんとアクセス制御できているかテストしている部分とか、コードのいろいろなところに書く

これによって、「本番でもページを見えるようにする」というgithubのTODOに対して、「どこを直すか」が明確になります。それによって、直すべき場所を忘れるという自体を無くす、もしくは一瞬で直すべきところを把握したいというイメージでした。

他にも「イベントが終わったら直したい」だったら、何をやるかという意味の「イベントのためのハードコードを消す」はgithubでissue化し、コード上のどこを変更するかを表すためにそれに関係するコードの部分にTODOコメントを書く、という使い分けになりそうです。


では「どこを直すか」という部分の細かい粒度のTODOもissueに書けばよいのでは、という話もありそうです。その場合、

  • 粒度が小さいのにgithubにわざわざ書いていくのは面倒
  • コードは日々変わるもので、issueのチェックリストでどこを直すか書いてあったとしてもそれが古くなってしまう可能性がある
    • gitの履歴などと連携して場所を指定するURLが作れるならそういうのを使うという手もある
    • しかしそこまでしなくてもコード上のコメントで問題なかろう

という判断をし、TODOコメントにしています。


その他ブコメで指摘がありましたが

  • ledsun 「本当は直したいけど、今回は〇〇な事情で見逃す」って理由の方を書いてほしい。期限までに直せなかったTODOコメントが残ったらどうするのか?
    • 本当は直したい、というような期限付きのコメントはTODOラベルをつけないので、本記事とは関係がなさそうです
    • しかし、TODOラベルがついていようと、そうでなかろうと理由は書くべきだと思います
  • bellonieta エディタのTODO一覧系プラグイン使えばわかるけど、FIXMEがバグ(絶対修正)、TODOがタスク(機能追加など)、FIXMEが「いつかやる」でしょ。
    • 知りませんでした
  • pmint そういうコメント内でこそLTSVの出番
    • 確かにそういうのを使っても良いかもしれませんね