無精で短気で傲慢なプログラマ続・これ、読みやすいの?

続・これ、読みやすいの?

2006-04-24（Mon）

perl - There's more than one best practice
> この点ははっきり言おう。こういう形でreturnできるのはperlの美点だと。

途中で return できるのは別に perl 独自の機能ではないですよね。「perl
なら許される」ということであれば、ちょっと理解できない。どんな言語で
あろうと、基本は出口はひとつがベストと考えます。ただし実際にプログラム
を組むと、例外処理・エラー処理が必要になるので、なかなかそうもいかない。
ではどういう return なら容認するかというと、わたしの場合は

sub func { return if $debug; ... }

という関数先頭付近で値のチェックをして return するタイプ。あるいは
関数の途中であっても、

sub func { ... $ret = subfunc1(); return if ( ! $ret ); $ret = subfunc2(); return if ( ! $ret ); $ret = subfunc3(); return if ( ! $ret ); ... }

といったように、後から読んだ人に
　　「この関数内ではこういう戻り方をする」
　　「だから新たなチェックをする場合は、この書き方を真似しろよ。
　　　if とか使うなよ」
とアピールしているなら OK。小飼さんのソースからは、書き手の意思やメッセージが
見えない (しいて言うなら「俺は return にはこだわらない」かな?)。書き手の意思を
こめられない場合は、if 文を使うべきだと思います。

で、この場合は return 云々以前に、そもそも関数の切り出し方がイマイチなのでは
ないでしょうか。search_result 関数は

　- Yahoo! にリクエストを投げてレスポンスを取得
　- レスポンスを解析
　- 取り出したデータに OL・LI 要素を付加して返す

という順序凝縮になっており、「この関数は何をしてるの?」という問いに対して、
「～です」とスパッと答えることができない。だから結果的に return に無理が出てくる。

最初に見たときは「OL 要素というオブジェクトを返す関数」かなと思ったのですが、
よく見ると

　('x hits', '<ol>', '<li>...</li>', '<li>...</li>', '</ol>')

と先頭にヒット数がついた配列を返しているので、ちょっと違う。

> そして、今回は関数化はもう一つの重要な利点を帯びている。
> それは、関数search_result()が、このサンプルコードの「ホット
> スポット」だからだ。読者が注意して読むべきなのは、そこだけ
> なのだ。あとは定数の設定、モジュールの宣言、結果の出力(print)
> といった「日常茶飯事」であり、読み飛ばしてかまわないところなのだ。

なるほど、そういう視点はありませんでした。しかし、ホットスポットと
いいつつも query が指定されていなければ return したり、HTML 生成を
行っていたり、いまいちホットスポットにしきれていない。トータルで
読みやすくなっているのか? というと、ちょっと疑問です。

結局のところ、仮にホットスポット化によってわかりやすくなるとしても、
return がいまいち、関数の切り出し方がいまいち、という結果になるなら、
関数化しないという選択肢も決して間違いではないでしょう。

なお、仕事でやるなら

$response = get_response() ($hit, @data) = parse_response($response)

と機能凝縮で分けます。しかし今回のソース短さでは引数・戻り値の受け渡しが
煩雑になるだけで、関数に切り出すメリットがありません。

で、わたしが一番気になるのは、雑誌の読者がこのソースを理解できるの?
ということです。まぁ、はてなユーザが相手なら問題ないのかもしれませんが、
たとえばラクダ本の存在すら知らなくて、CGI&Perlポケットリファレンス
で全部済ませてしまう人も多いわけです。

そういう人たちに map・URI.pm・CGI.pm をふんだんに盛り込んだソース
を与えても、意味がわからんと投げ出す人・ぱっと見て読もうともしない人が
少なからずいるような気がします。そうなると、Yahoo! API の使い方を伝える
という本来の目的が達成できない。

特に

return $xml->{'totalResultsAvailable'}, "hits", ol(map { encode_utf8 li(a({href=>$_->{'ClickUrl'}}, $_->{'Title'})) } @{ $xml->{'Result'} } );

を読んで、foreach ( @{ $xml->{'Result'} } ) でまわす書き方に書き
換えられない人は結構多いんじゃないでしょうか。

「そういう読者はターゲットにしていない」というのもひとつの考え方ですが、
あまりにもったいないなぁと。

最初のソースで解説しておいて、より美しいソースはコレだぜ! という展開
なら納得できますが、最初からリファクタリング後のソースを出すのはどう
なんでしょう。小飼さんのソースで編集からの OK が出ますか? (わたしは
雑誌執筆経験がないので、純粋な疑問です)

> それにしても惜しい。「無精で短気で傲慢なプログラマ」さんは、こうした
> 議論を進めるにあたって、サンプルコードを用意すべきであった。私のそれは
> きちんと動作検証までしてある(もちろんAPPIDは私のに変えてある)。少々
> 傲慢さが足りないのではないだろうか。

blog のタイトルで「俺って優秀だぜ」と自慢するのもおこがましいので当初は、
　「無精で短気で傲慢なプログラマを目指して」
としようかと思ったのですが、長かったので削ったのです。なので無精な
わけではないのです :-)

で、わたしの場合は、元のソースの方がわかりやすいと思いました。ただし
わたしなりの色を出すとすれば以下のようにします。おもしろみのないソースで
すみません。

[狙いと言い訳]
　- 目的は「Yahoo! API の使い方の解説」を読者に伝えること。
　- 読者を選ばないため、できる限りモジュールを使わない。
　- import せず LWP::Simple::get() と書くことで、モジュールを知らない読者が
　　混乱しないよう配慮。
　- デリファレンス結果を一度配列 @results に格納し、リファレンスをわかって
　　いない読者に配慮。
　- Model と View の分離をちょっとだけ主張してみる。
　- Model から View のデータ受け渡しの @click_urls と @titles は非常にかっこ
　　悪いが、Class::Struct を使ったり、クラスをうほどでもないので、ここは妥協。
　　(多分なにかしら適当なモジュールがあると思いますが、知らないので)
　- LWP::Simple::get に失敗した場合は致命的エラーなので die。
　　fatalsToBrowser で画面に表示。改造時の利便性を考慮。
　- parse_args は、インタフェース仕様書を兼ねている。プログラム内で必要な
　　入力はここで全部用意しろよ、途中で直接環境変数を参照したりするなよ、
　　サニタイズするならここでやれよ、という意図を込めたつもり。
　- 一度も動かしてません。構文チェックもしてません。申し訳ない。

#!/usr/local/bin/perl use strict; use warnings; use LWP::Simple; use XML::Simple; use Encode; use CGI::Carp qw(fatalsToBrowser); my $webapi_baseurl = 'http://api.search.yahoo.co.jp/WebSearchService/V1/webSearch'; my $myydn_appid = '..'; my $max_results = 10; my $query = parse_args($ENV{'QUERY_STRING'}); my $hit_num; my @click_urls; my @titles; if ( defined $query ){ # リクエストURIの生成 my $req_url = "$webapi_baseurl?appid=$myydn_appid&query=$key&results=$max_results"; # APIへリクエストを送信 my $yahoo_response = LWP::Simple::get($req_url) or die "$!"; # 取得したXMLをパースする my $xmlsimple = XML::Simple->new(); my $yahoo_xml = $xmlsimple->XMLin($yahoo_response, ForceArray=>['Result']); # 件数取得 $hit_num = $yahoo_xml->{'totalResultsAvailable'}; # URL とタイトルを取得 my @results = @{$yahoo_xml->{'Result'}}; foreach my $result (@results){ push(@click_urls, $result->{'ClickUrl'}); push(@titles, $result->{'Title'}); } } print "Content-type: text/html; charset=utf8\n"; print "\n"; print "<html><body>\n"; print "<form><input type='text' name='query'><input type='submit' value='search'></form>\n"; if ( defined $query ){ print "$hit_num hits\n"; print "<ol>\n"; for ( my $i=0 ; $i<@click_urls ; $i++ ){ printf("<li><a href='%s'></a></li>\n", $click_urls[$i], Encode::encode_utf8($titles[$i])); } print "</ol>\n"; } print "</body>\n"; print "</html>\n"; exit 0; #-------- sub parse_args { my ($query_string) = @_; my $l_query = undef; foreach ( split('&', $query_string) ){ my ($key, $value) = split('=', $_); $value =~ tr/+/ /; $value =~ s/(\W)/"%" . unpack("H2", $1)/ge; if ( $key eq 'query' ){ $l_query = $value; } } return $l_query; }