プロダクトのエンハンス開発をやっていると、1年以上前にコミットされてそのままになっているコードに出会うことがある。 プロダクトはリリース後1年以上経っている。なので最終コミットが2年以上前、とかもある。
で、コードの中にコメントが書いてあるときがある。 コメントに関しては割とシューキョー戦争な雰囲気がある。1
自分は実装の工夫などはコメントよりもPull Requestに書いている。作成側もこの機能は利用できる。 レビューする/される側どちらも見やすくて良いし、後から経緯を追いかける際にとても見やすい。issueとPull Requestで完結するのも良い。
…まあ基本人それぞれで好きにすればいいと思ってしまうのだが、推理が必要なコメントがあるとつらいなと思った。
推理が必要なコメント
推理が必要なコメントとは、「記述されている内容から色々と想像できてしまい、変更時に躊躇してしまう要素がある」コメントである。 「Aの入力があったとき」など、後から見た人でも再現できる状況が記載されていれば推理は必要ない。
でも、これがふわ〜と書かれている/省略されている場合がある。こうなると推理が必要で、とても疲れる。
例えば、明示的に型指定されている引数なのに型判定処理が記述されているような処理があったとする。2
function XXX(a: number){ // 画面からstringでaが渡されてくる場合がある const b = typeof a === 'number' ? a ? a : a.toNumber(); // 続きの処理あれこれ ... }
エンハンス開発でXXX関数を改良する必要が出てきた。この関数に対するUnitTestはない（良くはないがありがち）。3
これを見たとき、次のようなことを考えてしまう。
aは引数でnumber指定されているのだから、一時変数bの処理は不要では- でもコメントに
画面からstringでaが渡されてくる場合があると記述されているな - どんなときに発生するのだろう？
git blameで過去コミットやPull Requestにヒントがないか漁るか…（めっちゃ時間がかかる）
…と、コメントを見て疑問が浮かび、「なぜ型判定が必要なのか？」を推理しないと不安になってしまう。
大体こういうときはPull Requestやコミットメッセージがない or init みたいな内容がない状態とセットになっているパターンが多い。そうなると関数の呼び出し元を洗い出し、その処理を調べないとわからない。時間はかかるし精神的にもつらい。
どういうコメントなら推理しなくてもいいのだろうか？
推理しなくてもいいコメント
理由がしっかりと明記されていれば推理は必要ない。 例えばさっきのコメントは、こんな感じならつらくない。
function XXX(a: number){ // XXの仕様でstring型のパラメータが混入する場合がある issue#xxx const b = typeof a === 'number' ? a ? a : a.toNumber(); // 続きの処理あれこれ ... }
例外や困ったパターンがあるなら状況を再現する手順がどこかに欲しい。 作業issueやPull Request、関連チケットに記録を残し、コメントにURLを貼る・番号を追加してあるのが一番嬉しい。後から追いかけることができるので。
少し前はコミットログから追いかければいいや…と考えてきた。 しかし、途中でフォーマッタを導入して適用するとコミットログが上書きされてしまう。そうなると過去コミットを漁る必要が出てくるのでちょっとつらいなと思ってきた。
推理が必要なコメントはまだありそうなので、もう少し探してみたい。 「コメントがないと困るのでこうしましょう」という意見は良く見るが、実際に読まされた側の意見ってあまり見かけないなと思うのでした。
みんなどうしてるんでしょうね。
