taiyoh's memorandum

@ttaiyoh が、技術ネタで気づいたことを書き溜めておきます。

tokuhirom氏のリファクタリング見てすげーなー、と思った、って話

 typo氏からも「はよ」とか急かされたので、裏ですったもんだした挙句CPAN Authorになってしまいました。
 → http://search.cpan.org/~taiyoh/
 どうぞ宜しくお願いします><

 さて、本題ですが。
 今回Amon2::Web::Dispatcher::RouterSimple::Extendedというモジュールを上げたのですが、エントリ書いた時にgfx氏から「no strict 'refs'の状態でコードをガリガリ書くのはマズい」と指摘を受けました。僕も書いてる時からそれは薄々感じてはいたんですが、有効な策が打ち出せず、お茶を濁すような対応しか出来てなかったわけです。なので、追記の形で泣きついてみたら、翌朝tokuhirom氏からコメントをもらい、「こんなやり方がある」と紹介してもらいました。
 → https://github.com/tokuhirom/p5-Amon2-Web-Dispatcher-RouterSimple-Extended/blob/refactoring/lib/Amon2/Web/Dispatcher/RouterSimple/Extended.pm
 Amon2::Web::Dispatcher::RouterSimple::Extendedでは0.02で上記の氏のリファクタしたものを手動マージで採用し、0.03では更に自分なりに修正したり、バグフィックスを加えています。
 氏のリファクタリングしたソースを見て気づいたことで、一番考えなくちゃいけないのは、「なぜ"no strict 'refs'"をしなきゃいけないか」ってところだなと思いました。見ての通り今回のソースでは、呼び出し元に対して関数を追加しているわけですが、初期の僕の実装でやっていたのは、取り急ぎの追加であって、どのタイミングで関数を追加/変更するかについて、全然考えられてなかったのですね。場当たり的につけたり取り替えたりしてる。しかし氏のソースだと、追加する部分は一箇所にまとめられ、他の場所で関数を付け替えたりしていることは殆どない。submapのインスタンスの有無で、関数内の挙動を変化させている。あと、gotoをうまく使ってて、これはシラフでも全然思いつかなかったです。今後DSLっぽいのを書く時使えそうなので、意識してみたいと思いました。
 あと、0.03ではリファクタリングしてもらったソースに対して2点変更したところがあって、一つは単なるバグ修正です。もう一つはsubmapのインスタンスの持ち方についてで、local使えなくてかっこ悪くなってもいいから、モジュール内の変数でキャッシュさせる方式にして、submapの定義が終わったら明示的に消すようにすればいいや、という風にしました。そうすれば、no strict 'refs'はimportの中だけになるので。
 あとはpod周りだなー。今まで書き捨てみたいな感じでしかやってなかったので、突然cpanに上げることになって相当てんやわんやしておりました。ってか今もしてる。正直読みたくないけど、気づいたらボチボチ直していきます。。。
 それにしても、週末わざわざコメントしてもらったりリファクタリングまでしてもらい、ありがとうございました > tokuhirom氏