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";

回答の条件
  • 1人2回まで
  • 登録:2009/07/07 23:57:37
  • 終了:2009/07/14 16:28:34

ベストアンサー

id:Craftworks No.2

Craftworks回答回数20ベストアンサー獲得回数62009/07/08 01:18:34

ポイント50pt

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%   --
id:phji

>Perlベストプラクティス

これは良さそうですね

>push @a, @b;

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

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

>ベンチマークを取る癖

使い方を覚えます

2009/07/08 01:26:16

その他の回答(2件)

id:b-wind No.1

b-wind回答回数3344ベストアンサー獲得回数4402009/07/08 00:23:25

ポイント30pt

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";

こんなとこ?

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


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

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

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

id:phji
foreach my $page ( 1 .. $firends_pages )

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

2009/07/08 00:47:16
id:Craftworks No.2

Craftworks回答回数20ベストアンサー獲得回数62009/07/08 01:18:34ここでベストアンサー

ポイント50pt

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%   --
id:phji

>Perlベストプラクティス

これは良さそうですね

>push @a, @b;

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

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

>ベンチマークを取る癖

使い方を覚えます

2009/07/08 01:26:16

コメントはまだありません

この質問への反応(ブックマークコメント)

「あの人に答えてほしい」「この質問はあの人が答えられそう」というときに、回答リクエストを送ってみてましょう。

これ以上回答リクエストを送信することはできません。制限について

絞り込み :
はてなココの「ともだち」を表示します。
回答リクエストを送信したユーザーはいません