Last active
December 11, 2015 22:18
-
-
Save seki/4668150 to your computer and use it in GitHub Desktop.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
= るびまコード添削: WikiR、咳のターン。 | |
この連載は第一回の冒頭で説明されている通り、あるコードをテーマにした文通です。 | |
たいていの文通は人に見せるものではないのだけれど、今回は特別にみなさんに公開します。 | |
サラリーマンモードのコードレビューではにらむ、指差す、びっくりするなど言葉を使わなくてもわりと伝わりますが、今回は文通ですからなんとか言葉にしていきたいと思います。 | |
前回は須藤さんのターンでした。私の小さなWikiを題材に、須藤さんからコメントをいただきました。 | |
今回は私のターンです。須藤さんのコメントに私が言い訳していく番です。 | |
== 須藤さんのこと | |
須藤さんを知ったのはオーム社の森田さんからのメールです。 | |
私が昔(いまも)使っていたWebUIのライブラリを使ったアプリケーションを森田さんが見つけて | |
教えてくれたのです。当時、須藤さんは社長ではなく学生だったのではないでしょうか。 | |
たぶんそれと同じ頃にRWikiの開発に参加してくれたと思います。須藤さんは国際化やリファクタリングだけでなく、The RWikiの運営などめんどうなことも担当してくれました。昔過ぎて思い出せませんが、だいたいそんな感じです。 | |
RWikiは別の誰かとコードを書いた唯一のプロダクツ(語弊があるけど、誰かに頼って開発した、という意味)です。それ以来はどれもパッとしないというか相手にされてないいでいるし、まあ一人でもいいかなと思っています。そういうわけで、私が誰かとコードを書いたのは須藤さんが最後です。 | |
すごいでしょ。 | |
須藤さんとの作業はひっかかるポイントがよく似ていてとても楽しかったです。似ていても解決方法は全く違ったりすることもあり、誰かとやるのもいろいろ面白いなあ、と思い知らされましたね、そういえば。 | |
そうそう。須藤さんの話題と同じくらい重要なポイントがあります。私はいわゆるOSS的なかっこいい活動とは縁遠いということです。プルリとかパッチとかそういうモダンなソーシャルネットワークの世界を楽しんだ経験は少ないので、須藤さんが語られているよいパッチのスタイルなど関することのへの言及はしません。 | |
その分、ソースコードについてたくさん時間を割きますね。 | |
== ソフトウェアの構成 | |
まずソフトウェアの構成について解説されています。 | |
WikiRはdRubyサーバーとdRubyサーバーに接続するCGIで構成されています。これは、咳プロダクツではよくあるパターンです。咳フリークならみんな知っています。 | |
須藤さんはじめ多くの方はご存知かと思うのですが一応説明しますと、私はdRubyというRubyのプログラムならなんでもサーバにしてしまうライブラリを書きました。それ以来このような構成の、サーバによるアプリケーション本体(モデル?)とクライアントによるUIの組合せを用いたシステムを書くことが多いです。 | |
WebMVCがこういう風に流行る前には、モデルのプロセス/ビューとコントローラーのプロセスと分けて配置するように意識していましたが、最近はもう飽きてしまいました。 | |
アプリケーション自体をサーバとして考えるのは、Smalltalkのプロセスの様子や、AppleEvent/AppleScript時代のMacintoshのアプリケーションの利用、X11アプリケーション開発などの経験からです。まあ全然ちがうだろ!と思うかもしれませんが、私はよく似た何かを感じたんですよね...。 | |
なんの話でしたっけ。えっとソフトウェアの構成のことですね。須藤さんがおっしゃる通り、咳フリークならよくしっているいつもの構成でした。 | |
== 命名に関すること | |
私はプログラミングにおいて「名前重要」というのが好きじゃありません。なんだか名前だけが重要に聞こえるので「名前も重要」くらいが好みです。いろいろな判断をした結果として残されたのがコードです。名前をはその背景を知るひとつの手がかりで、読み手にとっては重要なヒントになります。 | |
=== 喩えた名前 | |
須藤さんはWikiR::Bookという名前についてコメントしています。 | |
ページを管理するクラスには他にも違う名前が考えられます。例えば「Wiki」という名前です。ページを全部集めたものがWikiだと考えれば適切な名前です。あるいは、「データベース」という名前です。ページが保存されている感じがします。「本」や「Wiki」ではどのように保存するかは気になりませんが、データベースという名前を使うとどのようにページを保存するかを意識している感じがしますね。 | |
咳さんは何かに例えた名前をつけます。作っているものはWikiですが、「ページが集まっているものと言えば本だよね」という連想をして「本」という名前をつけたのでしょう。このように何かに例えた名前をつけると愛着がわき、例えたものベースで説明するようになります。例えば、「ページの数が増えてきて処理に時間がかかるようになったね」というのではなく、「本が厚くなって重くなったね」というような感じです。自分が書いたソフトウェアに愛着がわくので一度は試してみるとよいでしょう((-参考: 「キラキラネーム」、「DQNネーム」-))。 | |
愛着もそうかもしれないけど、私はその文脈で識別し易いものを選ぶようにしている気がします。(今まで意識したことなかったですが、この文通のためにムリに考えています) | |
BookについてはWikiを作っているのにWikiって名前にしたらよくわからないし、辞書構造だからHashとかデータ構造を表すのも実装を狭めるし、なんとなくPageの集まりだからBookを選びました。もっと素敵な名前が出てくるかもしれないけど、以前書いたときもBookだったし、まあ惰性も強く働いたと思います。 | |
私がこういった名前の付け方を知ったのはTCLからです。TCLとはシマンテック社のTHINK PascalについていたTHINK Class Libraryです。tclとは関係ないです。 | |
私はこの小さくて速いコンパイラとクラスライブラリに夢中になりました。コンパイラに付属していたクラスライブラリのマニュアルを何度も何度も読みました。このクラスライブラリの日本語版のマニュアル、バージョンアップ後の英語版のマニュアルの二冊はまだ手元に置いてあります。もうボロボロになってしまったけれど捨てられません。 | |
喩えを使った名前はGUI部品ではよく見かけましたが、次のような機能を喩えた名前を知ってくすくすした覚えがあります。 | |
* CBartender: イベントを適切なオブジェクトにディスパッチする係 | |
* rainyDayFund / creditLimit / loanApproved: メモリが少ない時のための蓄え、借りれる最大サイズ、メモリ要求が承認されたかの状態 | |
はっ。そういえばプロセス間通信に使われるメカニズムの名前は比較的キラキラネーム(セマフォ、ランデブー、リンダとか)が多い気がしますね。 | |
=== set_srcのこと | |
RWikiの時もそうでしたが、咳さんは(({set_src}))という名前を使います。(({src=}))の方がRubyらしいのに、です。たぶん、「(({src=}))だと単純に値を設定するだけ」というイメージがあるけど、実際は「ソースを設定した後にHTMLに変換するというようにたくさんやっている」から(({set_src}))という別の名前にしているんじゃないかと思います。で、私はこの名前が好きではありません。 | |
同感です。私もset_srcは好きでありません。というよりも、まず思いつきません。当然、最初はsrc=で書き始めました。そうするとですね、 | |
@@ -29,11 +29,11 @@ class WikiR | |
class Page | |
def initialize(name) | |
@name = name | |
- set_src("# #{name}\n\nan empty page. edit me.") | |
+ self.src = "# #{name}\n\nan empty page. edit me." | |
end | |
attr_reader :name, :src, :html, :warnings | |
この記述に出会うわけです。「self.src = 」。どうですかこの奇妙さは! | |
ここまできてああそうか、src=はdRuby/irbなどの外部アプリケーションからの操作のためにとっておいて、内部は特別なことをしそう感のある奇妙な名前にしておこうかなあと思ったわけです。そんな思考を経て、10年前に書いたset_srcを「あのときもこんな気分だったのかなあ」と思い出しつつ拝借しました。src=以外の名前があったらそれにしたいと思います。 | |
=== 奇妙な省略形とattr_readerの位置 | |
(({km}))という変数名が気になります。これはKraMdownの略だと思いますが、このオブジェクトは(({Kramdown::Document}))オブジェクトなので(({km}))ではなく(({document}))が適切です。 | |
なんで省略形にkmを選んだのかよく覚えていないのですが、Kramdownを示す名前を使った理由は覚えています。私はよく暇つぶしにWikiを実装しますが、最初のバージョンは書式をもたないplain text(というよりHTML直書き)で始めます。その場合、フォーマッタはplainやhtmlだったりします。 | |
今回もそのように書き始めて、Kramdownのフォーマッタに取り替える時にkmとしました。その他のフォーマッタの採用を検討してもいましたし、この当時はdocでなくkmを選んだんでしょうね、たぶん。 | |
他にも(({src}))と省略しないで(({source}))にしたいとか、(({attr_reader}))は(({initialize}))の下じゃなくて上に書きたいとか、(({"# #{name}..."}))は(({default_source}))というメソッドとして括りだしたいとかありますが省略します。 | |
srcをsourceにするべきかは議論があると思います。 | |
常に省略しない名前が素晴らしいと誰かが言ってたみたいですが、自分の想定する読者の範囲のリテラシーに合わせて良いと思います。たぶん。私はそう信じているので、sourceはsrcでソースだと通じると考えました。Cでループを書くときの添字のiや、座標のx,y、stdin/stderr/stdout、またWikiWikiWebをWikiと略したりするのもわりと通じることが多いのではないでしょうか。 | |
そうそう。The dRuby Bookの訳者である井上さんからaryというのがイケテナイと指摘をうけたことがあります。aryはrubyのソースコードではポピュラーな省略形ですから、なんの躊躇もなく使用しています。通じなくてごめんなさい。 | |
さて、attr_readerの位置はどうでしょうか。Rubyではインスタンス変数を宣言する必要はなく、代入した瞬間に生まれます。多くの場合、initializeメソッドで初期値を入れることで宣言のように見せているのではないでしょうか。ですからattr_readerなど、インスタンス変数を操作するコンビニエンスメソッドを定義するのはinitializeの後が好みです。 | |
default値をつくるメソッドを分けるのは賛成です。その場合、Kramdownという書式を追い出してそのフォーマッタにデフォルト値を生成するメソッドを配置すると思います。 | |
== MonitorMixinのこと | |
(({MonitorMixin}))を(({include}))して、(({initialize}))で(({super()}))して、(({up}))の中の処理を(({synchronize do ... end}))しています。これはdRubyを使ったプログラムでよく見る処理です。 | |
と、(({MonitorMixin}))の説明をしてきましたが、私は(({MonitorMixin}))が好きではありません。(({initialize}))で(({super()}))するのがカッコ悪いなぁと思います。継承したときに(({initialize}))で(({super()}))するのは親クラスも初期化しないからといけないからだろうなぁとは思います。クラスをインスタンス化するときに(({initialize}))が呼ばれるというルールがあるからです。しかし、モジュールは(({initialize}))が呼ばれるというルールはありません。それなのに(({super}))を呼ばなければいけないのがカッコ悪いなぁと思う理由な気がします。 | |
MonitorMixinは私のコードでよく出現します。使用例などは須藤さんが示されている通りです。入れ子に呼び出してもロック可能なのでアプリケーションを書くのが楽です。 | |
実は私はMonitorMixinよりもMonitorが好きです。MonitorMixinが好きでない理由は@mon_owner, @mon_count, @mon_mutexの三つのインスタンス変数を定義してしまうところです。須藤さんの言う通り、superが必要なのもめんどくさいです。でもそれを補ってMonitorMixinを採用する理由があります。それはsynchronizeメソッドが定義されることです。 | |
class WikiR | |
class Book | |
- include MonitorMixin | |
def initialize | |
- super() | |
+ @monitor = Monitor.new | |
@page = {} | |
end | |
@@ -19,7 +18,7 @@ class WikiR | |
end | |
def []=(name, src) | |
- synchronize do | |
+ @monitor.synchronize do | |
須藤さんのパッチをごらんください。synchronizeは@monitorのメソッドのままです。これは、@monitorに関して排他制御することを意思表示しています。オブジェクト自身の排他制御ではなく、あるモニターに関する排他制御に見えます。あるオブジェクトの中で複数の関連しない区間が排他制御される可能性がある(つまりモニターが複数あるかもしれない、という心構えで読む必要がある)ということです。排他制御はある関心事について書かれるべきです。 | |
MonitorMixinがsynchronizeメソッドを定義してくれる場合、いま読んでいるオブジェクト全体が関心ごとであることを意思表示することになります。あるオブジェクトの中で本当に複数の関心事それぞれに排他制御が必要なケースで@関心事.synchronizeを導入するべきだと信じています。 | |
すると次のようなsynchronizeメソッドを導入すればいいじゃんと思いますよね。 | |
def synchronize(&blk) | |
@montiro.synchronize(&blk) | |
end | |
これを毎度定義するくらいなら、superとインスタンス変数が増えちゃうことを受け入れてMonitorMixinを使ってもいいかー、となるわけです。 | |
=== DefMethod | |
特に改善案はないのですが、前から(({DefMethod}))というモジュール名がカッコ悪いなぁと思っていることだけ書いておきます。 | |
はい、同感です。 | |
== 例外のこと | |
それでは、最近の書き方をしているところも見てみましょう。 | |
84 text = text.force_encoding('utf-8') | |
(({force_encoding}))は(({!}))がついていないので名前だけではわかりづらいですが、破壊的なメソッドです。そのため、戻り値を(({text}))に代入する必要はありません。これで十分です。 | |
うおおお。それは知りませんんでした。コメントありがとうございます。 | |
このメソッドにはそれよりも気になることがあります。それは、例外を握りつぶしているという事です。 | |
wikir.rb: | |
81 def do_request(req, res) | |
... | |
86 rescue | |
87 end | |
(({rescue}))の中でログを出力するなどした方がよいです。 | |
do_requestはWikiRの中で、外部から入ってくる怪しい入力を濾過してきれいな入力だけを扱うメソッドと位置づけました。怪しい入力を却下し、扱える入力だけを処理します。怪しい入力はいろいろな方法で怪しく、それに対してアプリケーションができることはほとんど残されていません。せいぜいログを吐いてユーザに言い訳するくらいでしょう。それくらいならhttpdのログにも手がかりはあるし、rubyの起動オプションで印字することもできます。(全ての例外が等しく重要であるなら、ですが) | |
このコメントは二通りの受け止め方ができます。一つは、HTTPのリクエストではログを残すべきである、もう一つは、例外を握りつぶすのはダメだ、です。 | |
前者の意図であれば、須藤さんの言う通りかもしれません。私はHTTPのサービスを仕事で書いた経験はありませんし、運用もしていませんから、経験豊富な須藤さんの意見が正しいと思います。 | |
後者の場合、私は「例外を握りつぶす」という表現がよくわかりません。例外をフィルタするのが悪で、ログするべきである、というニュアンスのことを言われることがよくありますが、私はそうは思いません。例外はちょっとかっこいい制御構文とエラー情報であって、他の戻り値やジャンプ命令と大差ありません。例外だけを特別扱いするのは奇妙な感じがします。 | |
アプリケーションはいつかどこかで例外を処理しなくてはなりません。例外はそれぞれの間で適切にフィルタされるべきだし、適切に発生させるべきだと思っています。ライブラリを設計するとき、全ての例外をそのまま通すのは簡単です。その処理の責任を呼び手に押し付けるわけですから。アプリケーションはライブラリの値域に対して正しく動くように書かなくてはなりません。例外の設計はエラー値のの設計でもあります。ライブラリはアプリケーションが書き易いような値域にすることを心がけるべきです。例外を返すということは、アプリケーションがそれを処理するべきだ、という意思表示であったり、一連の処理のそれぞれに対してエラーチェックすることなく、まとめてrescueでエラー処理するチャンスを与えるためであったりするでしょう。 | |
アプリケーションにとって役に立つ例外はなんでしょう。do_requestに関して言えば、ここで例外をあげてもアプリケーションができることはないし、積極的に例外を捨てることが正しいと判断しました。 | |
WikiRとは関係ありませんが、この言い訳をしているうちに例外を発生させないライブラリに対して、アプリケーションの記述を容易にするために、例外を発生させるラッパーを書いたことがあるのを思い出しました。 | |
* http://d.hatena.ne.jp/m_seki/20081203#1228239611 | |
これはKoyaというOODBの習作をしていて書いたものです。KoyaのストレージにはTokyoCabinetを利用しました。TCでは「例外を発生させず、戻り値でエラーを表現する」という方針を採用しており、エラーが起きるかもしれない呼び出しに関しては戻り値をチェックする必要があります。 | |
アプリケーションで全てのTCの操作に対して戻り値の検査をするのは退屈ですよね。たとえばトランザクションの処理のようなものを想像してください。トランザクション中で行うTCの操作の全てについていちいち検査するのではなく、rescueでまとめて処理するとアプリケーションのコードのシンプルになります。 | |
class BDBError < RuntimeError | |
def initialize(bdb) | |
super(bdb.errmsg(bdb.ecode)) | |
end | |
end | |
class BDB < TokyoCabinet::BDB | |
def exception | |
BDBError.new(self) | |
end | |
def cursor | |
TokyoCabinet::BDBCUR.new(self) | |
end | |
def self.call_or_die(*ary) | |
file, lineno = __FILE__, __LINE__ | |
if /^(.+?):(\d+)(?::in `(.*)')?/ =~ caller(1)[0] | |
file = $1 | |
lineno = $2.to_i | |
end | |
ary.each do |sym| | |
module_eval("def #{sym}(*arg); super || raise(self); end", | |
file, lineno) | |
end | |
end | |
call_or_die :open, :close | |
call_or_die :tranbegin, :tranabort, :trancommit | |
call_or_die :vanish | |
end | |
call_or_dieというクラスのメソッドにSymbolを渡すと、戻り値が異常なら例外を発生させるメソッドを定義します。 | |
こんな風に、処理はうまくいかなかった(うまくいかなかったらfalseを返すという仕様のメソッドだと考えると、メソッドはうまく動いているとも言えるんだよなー)けど例外があがらないケースって多々ありますよね。この場合はどう考えたらいいんでしょう。「例外」が特別なのか戻り値がfalseなのも特別なのか、いや全てのメソッドの戻り値はログされるべきだとか、いろいろ考えられますが、こういうのってあるルールで機械的に思考する類のものではなく、その状況に応じて対応をテキトーに選ぶものではないでしょうか。 | |
あっ、ここばっかり言い訳が長くなりました。苦しいので先に進みます。 | |
== WEBrick::CGIのこと | |
WEBrick::CGIはここ五年くらい(もっと?)意識して使ってます。 | |
CGIやWEBrickサーブレットなどのアプリケーションインターフェイスはよく似ているけど、ちょっとずつ違ってます。たいていそういうのをみるとインターフェイスをそろえてみたくなるのが人情です。私も20世紀にそういうのをやってたんですが、全部のインターフェイスをそろえるのめんどくさいんですよね。細かい仕様とか調べるの苦手だし。そこでCGIでもなんでもWEBrickのインターフェイスにそろえることができそうだと気付いておねだりして作ってもらったのがWEBrick::CGIです。よく似たアイデアのものも多いかもしれないけど、gemなしですぐ使えるのが魅力です。 | |
WikiRもCGIでないところに配置されたとしても、WEBrickなアプリケーションインターフェイスを変更することなく動作させることができます。 | |
== index.rbのこと | |
もっとも、index.rbのように非常に小さいCGIはRailsをCGIで起動するようなケースと比べて軽いため、実用的な速度を持ちます。イントラネットで使用するには十分なことも多いです。 | |
index.rb: | |
#!/usr/bin/ruby | |
require 'drb/drb' | |
DRb.start_service('druby://localhost:0') | |
ro = DRbObject.new_with_uri('druby://localhost:50830') | |
ro.start(ENV.to_hash, $stdin, $stdout) | |
その人のコードを見続けていると、この人はどうしてこんなコードを書いたのか、というのがわかってきます((-想像できるようになってくるだけで、わかってはいないかもしれません。-))。昔、咳さんのコードを見ていた私からすると「このコードはいろんなプロダクツで使いまわしているコードだろうなぁ」と感じます。 | |
ちゃんと書いた咳さんのコードでは「(({ro}))(たぶん、Remote Objectの略)」という名前を使いません。咳さんなら「(({wikir}))(リモートにあるどのオブジェクトを触るかがわかる名前。一見、ローカルのオブジェクトに見える名前をつけるはずです。wikir.rbの最後を見るとリモートにあるのは(({WikiR::UI}))ですが、(({ui})))にはしないはずです。リモートのオブジェクトを提供する側は公開用に(({WikiR::UI}))というオブジェクトを用意していますが、使う側からすればWikiサービスを使いたいだけなので、それがUI用のやつかどうかなんて気にしたくありません。)」や「(({front}))(dRuby本でも使われている伝統的な名前)」といった名前を使うはずです。 | |
その通り、このコードは使い回しです。相手が特定の何かであることを意図しないのでremote objectとしています。thereでもよかったんだけど。 | |
最近のテストコードはもっとひどくて、ファイル名をそのままポート番号にしています。外部のサービスでこんなことをする人はいないと思うけど、紹介しておきます。 | |
54321.rb | |
require 'drb/drb' | |
DRb.start_service('druby://localhost:0') | |
ro = DRbObject.new_with_uri('druby://localhost:' + File.basename($0, '.rb')) | |
ro.start(ENV.to_hash, $stdin, $stdout) | |
== まとめ | |
須藤さんからいただいたコメントにたくさん言い訳しました。 | |
気になるポイントは近いのに選んだコードが多いことに(慣れてたけど)やっぱり驚きました。 | |
意識の高い読者が多いところで述べるのは怖いのですが、最後に。 | |
コードのどうでもいいところはどうでもよく書く。それも意図の表現のひとつであると思っています。 | |
次回は別のペアでやってくださいー。 | |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment