変なふうにコードに手を入れられたことがあった。

もともとあったのは、リクエストの情報を持ったオブジェクトを引数に取り、その内容の整合性をチェック、真偽値を返す関数。仮に名前を verify とする。

とある改修のため、このコードに手が入れられ、自分はその差分をレビューした。内容を見て違和感。関数のインターフェースがすっかり変わってしまっていた。

• 真偽値を返すはずが、別のデータを返すようになっていた。そのデータはその後の処理で使われている。
• もとの処理には引数の情報をもとにデータベースをひいて、データの整合性をチェックする部分があった。この結果をあとの処理で使いたかったらしく、戻り値としてそれが使われていた。
• そのうえ、必要なデータだけではなく、データベースを引いた結果がそのまますべて返されている。

不慣れなコードをさわると、つい一番簡単な手段をえらんでしまい、全体の整合性を損なってしまうのはよくあることだ。それにしても、関数名と実態があわなくなってしまっていて、これはどうしたものかと思わなくもなかったが、対策を考えたほうが前向きだ。

こうした事態をふせぐには、よくない変更のハードルを高くしておく。そういう改修をしようとしても、めんどうくさくなるようにしておくことが、ひとつ考えられるだろう。今回のケースには次の点が欠けていて、結果として変な方向に改修が向かうような力学が働いていた。

• まず関数名がよくなかった。真偽値を返す関数なので is_valid のように、is_*、has_* という名前にすべきだった。こうして名前で関数の意図を補強して、変な使い方をしようとしても違和感がでやすくする。
• つぎはテストだ。もっと外側のテストはあったが、関数レベルの単体テストが抜けていた。テストを書いてあると、インターフェースの変更をそう安直にしようとは思わないはずだ。もとの意図も伝わるし、変更箇所も多くなるので否が応でもよく検討するようになる。
• ドキュメントも、今回の関数のものはなかった。ドキュメントを書いておくことでも、意図の明確化と、変更点が多くなって慎重になるという効果が期待できる。

まとめると、元のコードの意図が明確に伝わるようにすることと、インターフェースなど重大な変更にはそれなりの手間がかかるようにしておく、ということになるだろうか。

そもそもインターフェースの変更はめんどうくさいもののはずだ。テストもドキュメントもなかったとしても、それをよびだしている箇所が複数あるのが普通だからだ。ひるがえって、はじめの設計がまちがっているとその修正がとてもめんどうになるとも言える。インターフェースを決めるのは慎重に、ということを今一度思い返す出来事だった。