Nekoya Press

2013-02-12 Tue 08:23

ダメなのはcheck_xxxだけではない

id:Yamashiro0217さんのcheck_xxx がなんでダメなのか - Yamashiro0217の日記が面白かったので、少し掘り下げてみる。

第一印象は「いくら何でも例が恣意的すぎ。check_XXX言うな言いたいだけちゃうんか」だったが、現実は非情であり、こういうコードを押し付けられたこともあるので決して大袈裟ではなかった。

こういう名前が付く場合というのは、往々にしてメソッドの目的が定まっていないことが多い。そこに、isvalidXXXのような名前を持ち込んで戻り値の型を規定してしまうことは有効なアプローチである。やる人は名前に関わらずどこでもexitするが、それは個別に教育していくしかない。

そんな訳でcheckXXXは確かに避けたい名前だが、isvalidxstateやisvaliduseraboutxもやはりよろしくない。checkuserxvalueandifinvalidupdatexandredirectはネタとしても、その方向性には危険な香りがする。

メソッド名が長くなるのはクラスが自身の責務を外部に放り投げることにつながるからだ。isvaliduseraboutxはその分かりやすい例で、対象となるオブジェクトがUserなのかXなのか分からない。

これは、$user->hasValidX()もしくは$x->isValid()として実装されるべきだろう。メソッド名に対象となりうる物が複数出てきたら、その時点でクラスの責務が揺らいでいるのでリファクタリングした方がよい。

また、メソッド名に具体的な条件を表す文言が出てきた場合は、クラスではなくメソッドが責務を持っている可能性が高いので、これも怪しい匂いである。「メソッド名が処理内容そのままで、何も抽象化されていない」が代表的な症例だろう。

と、ここまで読んで何かおかしいと思ったら、サンプルコードに一つもクラスが無いことに気付いた。オブジェクトが設計されていないので、メソッド名だけを整えたところで根本的な問題が解決しないのだ。

メソッドはオブジェクトの公開インタフェースなので、メソッド自体の名前ももちろん大切だけど、その前提となるオブジェクトのクラス設計の方がもっと大切なので気を付けましょう。クラス設計が不適切だと、名前の付けようもなかったりするし。

元ネタでは簡略化のためにあえてクラスを省略したのかも知れないが、メソッドはオブジェクトと対になって意味を持つものなので、やはり省略すべきではない。

なお、オブジェクトに従属していないPHPの関数はメソッドではなく「サブルーチン」と呼びたい。なので自分には、このサンプルのget_XXXをメソッド名と呼ぶこと自体に違和感がある。

そして、get_user()という名前はサンプルコードにはありがちだが、この名前も闇が深いと思っている。これについてはまた別途取り上げたい。

ちなみに、個人的にはcheck_XXXと聞いて最初に思い浮かぶのはNagiosプラグインなので、boolで成否が返ってくることが期待される。が、Nagiosプラグインもwarn/crit/unknownなど様々な戻り値があるので、確認する対象によっては違う型を返すことがあり、やはり適切でない。

nekoya.github.io