こういう意見を待っていた。
無精で短気で傲慢なプログラマ | これ、読みやすいの?わたしなんかよりよっぽど perl を知っている人なのだろうから機能的な 点についてはコメントしないが、はたしてこの添削後のコードはきれいなのか?
実は、私が最初にRefactorしたものは以下のような感じだった。
1: 2: 3: 4: 5: 6: 7: 8: 9: 10: 11: 12: 13: 14: 15: 16: 17: 18: 19: 20: 21: 22: 23: 24: 25: 26: 27: 28: 29: 30: 31: 32: 33: 34: 35: 36: 37: 38: 39: 40: 41: 42: 43: 44: 45: 46: |
#!/usr/local/bin/perl -T
use strict;
use warnings;
use Readonly;
use URI;
use CGI;
use LWP::Simple ();
use XML::Simple;
use Encode qw/encode_utf8/; # to drop utf8 flag from XML::Simple
Readonly my $WEBAPI_BASEURL
=> 'http://api.search.yahoo.co.jp/WebSearchService/V1/webSearch';
Readonly my $MYYDN_APPID => 'YahooDemo';
Readonly my $MAX_RESULTS => 10;
my $q = CGI->new;
sub search_result{
my $q = shift;
return unless $q->param("query");
my $uri = URI->new($WEBAPI_BASEURL);
$uri->query_form( appid => $MYYDN_APPID,
query => $q->param("query"),
results => $MAX_RESULTS );
my $response = LWP::Simple::get($uri) or return;
my $xml = XML::Simple->new->XMLin($response, ForceArray=>['Result']);
my @result = ($xml->{'totalResultsAvailable'}, "hits");
push @result, "<ol>";
for my $r ( @{ $xml->{'Result'} } ){
push
@result,
encode_utf8(sprintf qq(<li><a href="%s">%s</a></li>),
$r->{'ClickUrl'}, $r->{'Title'});
}
push @result, "</ol>";
return @result;
}
print
$q->header(-charset => 'UTF-8'),
$q->start_html(-lang=> 'ja', -title=>$ENV{SCRIPT_NAME}, -encoding=>'UTF-8'),
$q->start_form(),
$q->textfield("query"),
$q->submit(-value=>"search"),
$q->end_form(),
search_result($q),
$q->end_html();
|
これは、二つの点で前のEntryより優れている。
- よりオブジェクト指向のため、Namespaceの浪費が少ない。
上記関数の引数がなく、$q がグローバル変数化しているところ。
に通じる。ただし、use CGI qw/:standard/;した時に有効になるparam()は、グローバル変数ではなく、import()されたsubなので、この指摘は適切さに欠ける。 - map()を使うより敷居が低い
mapは実に強力な武器であり、中級以上のperl programmerであれば是非使いこなせるようになって欲しいものであり、かつ今回の使い方は比較的idiomaticなものではあるが、下から上へと読まねばならないという難点がある。この辺はrubyの
array.map{ |r| .... }が美しく書け、両方知っているprogrammerがrubyに惚れる一因でもある。perl 6ではこの辺が実にelegantに見直されており、該当部分は($xml.<totalResultsAvailable>, "hits", $xml.<Result>.map:{ $q.li($q.a({ href=>$_.<ClickUrl> }, $_.<Title>)) });と実に自然に書ける。
それ以外の点に関しては、問題ないだろう。
途中で節操なく return しているところ。
この点ははっきり言おう。こういう形でreturnできるのはperlの美点だと。これに限らず、loopの中でnext if ...やlast if ...を使って余計なインデントを減らすのは、まさに
解説が目的の場合は読者の脳内スタックをなるべく消費しない
ことに通じるからだ。インデントを過度に使うのを慎むべきなのは、まさにそれが理由なのだから。
つまり過度に関数化しない方がよいと思う。
そして、今回は関数化はもう一つの重要な利点を帯びている。
それは、関数search_result()が、このサンプルコードの「ホットスポット」だからだ。読者が注意して読むべきなのは、そこだけなのだ。あとは定数の設定、モジュールの宣言、結果の出力(print)といった「日常茶飯事」であり、読み飛ばしてかまわないところなのだ。
そうそう。なぜこのRefactor版を使わなかったかと言えば、それはこのサンプルコードがまさに書籍という容量制限の厳しいメディアに掲載されているからである。そこにおけるbest practiceは、smallest practiceであることも強く要求される。しかもこのスクリプトは、実際にはずっとカラム幅が少ないのだ。
best practiceはたった一つの冴えたコードを要求するものでは決してないのだ。
Damianや私がPerlを愛してやまない理由、それはbest practiceもまた一つでないことをこれほど高らかに主張する言語が未だにないからだ。Perlならフォーマルにもカジュアルにもできる。丁寧に話す事も毒づくこともできる。しかしどの場合にも「かっこよく」あることを目指すのが、best practitionerの条件というものではないだろうか。
それにしても惜しい。「無精で短気で傲慢なプログラマ」さんは、こうした議論を進めるにあたって、サンプルコードを用意すべきであった。私のそれはきちんと動作検証までしてある(もちろんAPPIDは私のに変えてある)。少々傲慢さが足りないのではないだろうか。
Dan the Lazier, More Impatient, and More Hubristic Perl Monger
同感です。
「少々傲慢さが足りないのではないだろうか。」の部分は、とくにcool。