添削その2

http://kokogiko.net/m/archives/001966.html
別に呼ばれてへんけど、なんかな、これはアカンと思った。

my %analyze = (
    'Pattern 1' => 'Pattern 1',
    'Pattern 2' => 'Pattern 2',
    'Pattern 3' => 'Pattern 3',
        ....
    'Pattern N' => 'Pattern N',
);

my @analyzekey = keys %analyze;
my @analyzeval = map { $analyze{$_} } @analyzekey;

my $reg = "(?:".join("|",@analyzekey).")";
$reg = qr/$reg/;

while (my $log = $logs->readline()) {
    if (($log->{ua}) && ($log->{ua} =~ /$reg/)) {
        my ($pattern) = map { $analyzeval[$_-1] } grep { eval "\$$_" } (1..$#analyzeval+1);
        # Do hoge hoge for $pattern
    }
}

あんな、まずな

my @analyzekey = keys %analyze;
my @analyzeval = map { $analyze{$_} } @analyzekey;

ここやねんけどな、values いうの使ったらええと思ってん。

my @analyzeval = values %analyze;

結果一緒やで。

my $reg = "(?:".join("|",@analyzekey).")";

いや、これはちゃうやろ。
あとでパターンマッチするんには後方参照出来るようにしとかなあかんのとちゃう？

(?:Pattern 1|Pattern 3|Pattern N|Pattern 2)

ってなんで。

(?:(Pattern 1)|(Pattern 3)|(Pattern N)|(Pattern 2))

こうしたらなあかんやろ。

my $reg = sprintf('(?:%s)', join('|', map { "($_)" } @analyzekey));

こんなんしといたらええんとちゃうか？

$reg = qr/$reg/;

これな、あとで

$log->{ua} =~ /$reg/

こんなんしてんねやったらやる意味ないんとちゃうん？

my ($pattern) = map { $analyzeval[$_-1] } grep { eval "\$$_" } (1..$#analyzeval+1);

これはさすがのワシもドンびいたわ。
eval() て重いやろ。
重いループの中で eval() は絶対やったらあかんと思うで。

(1..$#analyzeval+1);

scalar() 使うとかな、そもそも scalar コンテキストでやると要素数になるから

(1 .. @analyzeval);

でいけるやろ。
あと、一行で書いてなんか小綺麗にまとまった思ってるかも知れんけどな、

my ($pattern) =

一個しか要素を望んでいないのにな

grep { eval "\$$_" } (1..$#analyzeval+1);

これ、要素数ぶんループして eval() してるんちゃうか？
一個しか値が欲しくないにもかかわらず、$1 が true になっても、N 個ぶんループするやろ。あとずーっと undef っぽい値を eval() すんねん。
「安易なループ」がうんぬん言うとる君がやってたらアカンやろ。
もったいないな。
そもそも eval() はあかん。
せっかくパターンマッチしたら参照配列が出来るんやから、それつこたらええと思うで。

$reg = qr/$reg/;

while (my $log = $logs->readline()) {
    if ($log->{ua}) {
        my @jitensha = $log->{ua} =~ $reg;
        next unless @jitensha;
        my $pattern;
        for my $i (0 .. $#jitensha) {
            if (defined $jitensha[$i]) {
                $pattern = $analyzeval[$i];
                last;
            }
        }
        # Do hoge hoge for $pattern
    }
}

こうしといたら、一応全部ループもせんし eval() とかもせんから、きっと速いやろ。

いや、言うてもワシの自転車の速さにはかなわんけどな。

あー、大塚さんみたいに嫁はん欲しいなー。

※このエントリはJ'sHeadPetによる自動投稿です