人力検索はてな
モバイル版を表示しています。PC版はこちら
i-mobile

Perl の書き方に関する質問です。
以下のプログラムがPerl としてあるいは一般的なプログラムの書き方として間違っている、ダサい、遅い等がありましたら教えてください。私が気になっているのは

・配列を結合している @array = (@array, @hoge) という箇所
・差集合を求める際にモジュールを使っている箇所(モジュールなんて使わなくても簡単に書けるのではないか)

です。Twitter の Following の中で Followers にいない id を探す動作をします。よろしくお願いします。

use strict;
use warnings;
use Net::Twitter;
use List::Compare;

my ($username, $password) = ('','');
my $twit = Net::Twitter->new(username => $username,
password => $password);

my ($friends_count, $followers_count) = sub {($_[0]->{'friends_count'},
$_[0]->{'followers_count'})}->($twit->show_user($username));
my (@friends, @followers);


for(my $page=1;$page-1 < ($friends_count/100);$page++){
@friends = (@friends, map {$_->{screen_name}} @{$twit->friends({page=>$page})});
}

for(my $page = 1;$page-1 < ($followers_count/100);$page++){
@followers = (@followers, map {$_->{screen_name}} @{$twit->followers({page=>$page})});
}

my $lc = List::Compare->new(\@friends, \@followers);

print join(' ', $lc->get_Lonly), "\n";

●質問者: phji
●カテゴリ:コンピュータ ウェブ制作
✍キーワード:e+ E-1 friends hoge LC
○ 状態 :終了
└ 回答数 : 3/3件

▽最新の回答へ

1 ● b-wind
●30ポイント

Perl としてあるいは一般的なプログラムの書き方として間違っている、ダサい、遅い等

答えにくい質問だな。Perl はいろいろな書き方が出来るのが最大の特徴でもあるから。

もちろん非効率だったり、読みにくいソースは論外だが。


ざっと見たところ、さほど大きなミスは犯していないと思う。

あえて採点すると、自分ならこう書く。

use strict;
use warnings;
use Net::Twitter;
use List::Compare;

my ($username, $password) = ('','');
my $twit = Net::Twitter->new(
 username => $username,
 password => $password,
);

my $twit_user = $twit->show_user($username);
# $twit_user のチェックルーチンがあれが入れた方が良いかも?

my $friends_count = $twit_user->{'friends_count'} || 0,
my $followers_count = $twit_user->{'followers_count'} || 0;
my $firends_pages = int ( $friends_count / 100 );
my $followers_count = int ( $followers_count / 100 );


my (@friends, @followers);

foreach my $page ( 1 .. $firends_pages ){
 push @friends, map { $_->{screen_name} } @{$twit->friends( { page => $page, } )};
}

foreach my $page ( 1 .. $followers_pages ){
 push @followers, map { $_->{screen_name} } @{$twit->followers( { page => $page }, )};
}

my $lc = List::Compare->new(\@friends, \@followers);
print join(' ', $lc->get_Lonly), "\n";

こんなとこ?

内容的にはほぼ等価だけど、多少冗長気味に書いた方が意味が分かりやすくていいよ。


モジュールなんて使わなくても簡単に書けるのではないか

モジュールを使うことで分かりやすくなるのなら、その方がいいです。

モジュールの使い方自体が難しく、やってることが意味不明になるなら別ですが。

◎質問者からの返答
foreach my $page ( 1 .. $firends_pages )

この書き方の方が良さそうですね


2 ● Craftworks
●50ポイント ベストアンサー

for (my $page = 1;$page-1 < ($followers_count/100);$page++)

より

for my $page ( 1 .. $firends_pages )

の方が良いですね。

その他は、PBP を(ある程度)参考にするといいと思います。

http://perl-mongers.org/2008/05/perl-bestpractice_1.html

@a = (@a, @b);

は、普通に

push @a, @b;

でいいと思います。後者は追加だけなので速いです。メモリのコピーはコストがかかりますので。

ベンチマーク結果です。速度が気になったときはベンチマークを取る癖を付けると良いと思います。

ベンチマークコードも貼っておきます。

use strict;
use warnings;
use Benchmark ':all';

my @alpha = ('a' .. 'z');
my @num = (1 .. 100);

cmpthese(timethese(10000, {
 'subs' => sub {
 my @a = @alpha;
 my @b = @num;
 @a = (@a, @b);
 },
 'push' => sub {
 my @a = @alpha;
 my @b = @num;
 push @a, @b;
 },
}));

結果です。2倍弱速いです。

Benchmark: timing 100000 iterations of push, subs...
 push: 4 wallclock secs ( 4.00 usr + 0.01 sys = 4.01 CPU) @ 24937.66/s (n=100000)
 subs: 7 wallclock secs ( 6.34 usr + 0.00 sys = 6.34 CPU) @ 15772.87/s (n=100000)
 Rate subs push
subs 15773/s -- -37%
push 24938/s 58% --
◎質問者からの返答

>Perlベストプラクティス

これは良さそうですね

>push @a, @b;

質問後ですが、添字に対して map をすれば速くなるのではとちょっと思いました。

@friends = map {map {$_->{screen_name}} @{$twit->friends({page=>$_})}} 1 .. ($friends_count/100)+1; 

>ベンチマークを取る癖

使い方を覚えます


3 ● 黒霊月広
●0ポイント

........

関連質問


●質問をもっと探す●



0.人力検索はてなトップ
8.このページを友達に紹介
9.このページの先頭へ
対応機種一覧
お問い合わせ
ヘルプ/お知らせ
ログイン
無料ユーザー登録
はてなトップ