古いスクリプトを見直してみる - 2 -

今日はISBNのハイフン編集編

実際のコード

  • 最初に書いたサブルーチン(を多少書き直したもの)
#!/usr/bin/perl
use lib '/home/natu-n/perl/lib/perl5/site_perl/5.8.8';
use lib '/home/natu-n/local/lib/perl5';
use strict;
use warnings;
use Readonly;
use Data::Dumper;
use List::MoreUtils qw( first_index );
use Benchmark;
#
Readonly my $DASH => '-';
Readonly my @RECORD_LAYOUT => (
    '@0 A1 A2 A6 A1',
    '@0 A1 A3 A5 A1',
    '@0 A1 A4 A4 A1',
    '@0 A1 A5 A3 A1',
    '@0 A1 A6 A2 A1',
    '@0 A1 A7 A1 A1',
);
Readonly my @SEP => (
     20,
     70,
     87,
     90,
     95,
    100,
);

my $isbn  = 4798109401;
my $count = 100_000;

timethese($count,
    {
    'TEST1' => "&adddash_1($isbn)",
    'TEST2' => "&adddash_2($isbn)",
    'TEST3' => "&adddash_3($isbn)",
    }
);
sub adddash_1 {
    my $isbn = shift;
    my $pub = substr($isbn, 1, 2);
    my $pos;
    if ($pub < 20) {
        $pos = 2;
    }
    elsif ($pub < 70) {
        $pos = 3;
    }
    elsif ($pub < 87) {
        $pos = 4;
    }
    elsif ($pub < 90) {
        $pos = 5;
    }
    elsif ($pub < 95) {
        $pos = 6;
    }
    else {
        $pos = 7;
    }
    return join $DASH
                , substr($isbn, 0         , 1          )
                , substr($isbn, 1         , $pos       )
                , substr($isbn, ($pos + 1), (8 - $pos) )
    ;
}

とりあえず思いつくままにif〜elsifの連続で書いてみた

  • 一回目の見直し版
#先頭の宣言部分は同じ

sub adddash_2 {
    my $isbn = shift;
    my $pub = substr($isbn, 1, 2);
    my $pos = ($pub < 20) ? 2
            : ($pub < 70) ? 3
            : ($pub < 87) ? 4
            : ($pub < 90) ? 5
            : ($pub < 95) ? 6
            :               7
            ;
    return join $DASH
                , substr($isbn, 0         , 1          )
                , substr($isbn, 1         , $pos       )
                , substr($isbn, ($pos + 1), (8 - $pos) )
    ;
}

3項演算子を使うとちょっと知ってる感じ?で記述してスッキリ

  • 二回目の見直し版
#先頭の宣言部分は同じ
sub adddash_3 {
    my $isbn   = shift;
    my $pub    = substr($isbn, 1, 2);
    my $ix     = first_index { $pub < $_ } @SEP;
    my @w_isbn = unpack $RECORD_LAYOUT[$ix], $isbn;
    return join $DASH, @w_isbn;
}

List::MoreUtilsのfirst_indexを使って条件に合致する配列の先頭のインデックスを求め、インデックス番目のレコードレイアウトでハイフンを挿入
#個人的にはしてやったりって思っていたけど><

ベンチマーク

  • テストコード
  • 結果
Benchmark: timing 100000 iterations of TEST1, TEST2, TEST3...
     TEST1:  1 wallclock secs ( 0.95 usr +  0.00 sys =  0.95 CPU) @ 105785.12/s (n=100000)
     TEST2:  1 wallclock secs ( 0.93 usr +  0.00 sys =  0.93 CPU) @ 107563.03/s (n=100000)
     TEST3:  3 wallclock secs ( 3.25 usr +  0.00 sys =  3.25 CPU) @ 30769.23/s (n=100000)

思った通り圧倒的に遅い、対象を世界の書籍全部にして条件をYAMLとかで外に出すとかなら可読性が上がっていいのだけど><
まぁ普通なら間をとって二番目の方向ですかね?

備考

次は「これってXPAth式どう書くの」または「あ、こういう表現YAMLじゃ出来ないんだ」のどちらかで

追記

一つ目の二つ目のスクリプトに問題があり正しい評価になってませんでした
最後のチェックデジットの部分を返していなかった(中間検索用に)ので同じ条件ではないです
誤)

    return join $DASH
                , substr($isbn, 0         , 1          )
                , substr($isbn, 1         , $pos       )
                , substr($isbn, ($pos + 1), (8 - $pos) )
    ;

正)

    return join $DASH
                , substr($isbn, 0         , 1          )
                , substr($isbn, 1         , $pos       )
                , substr($isbn, ($pos + 1), (8 - $pos) )
                , substr($isbn, 9         , 1          )
    ;

それと三つ目のスクリプトに無駄な部分があったのでちょっと修正、一回@w_isbnに展開する意味が無いですね

sub adddash_4 {
    my $isbn   = shift;
    my $pub    = substr($isbn, 1, 2);
    my $ix     = first_index { $pub < $_ } @SEP;
    return join $DASH
                , (unpack $RECORD_LAYOUT[$ix], $isbn)
    ;
}

もう一回計ってみる

Benchmark: timing 100000 iterations of TEST1, TEST2, TEST3, TEST4...
     TEST1:  1 wallclock secs ( 1.16 usr +  0.00 sys =  1.16 CPU) @ 86486.49/s (n=100000)
     TEST2:  1 wallclock secs ( 1.15 usr +  0.00 sys =  1.15 CPU) @ 87074.83/s (n=100000)
     TEST3:  4 wallclock secs ( 3.25 usr +  0.00 sys =  3.25 CPU) @ 30769.23/s (n=100000)
     TEST4:  3 wallclock secs ( 3.11 usr +  0.00 sys =  3.11 CPU) @ 32160.80/s (n=100000)

こうして見ると結構文字列や配列の操作ってコストが掛かるんだなぁとあらためて思う