良いコードを参考にしましょう①

日付に関する処理を行うコードをいかに書くか。

my $date = $ARGV[0];

&get_date_temp($date);

sub get_date_temp{
    my ($date) = @_;
    if (!$date){
        my $dt = DateTime->now(time_zone => 'Asia/Tokyo');
        $dt->add( days => 1 );
        $date = $dt->ymd('-');
    }
    unless ($date =~ /\d{4}-\d{2}-\d{2}/){
        print "Please enter the correct date format.\n";
        return undef;
    }


    my $api_data = &get_api_data;
    my $temp_ref = &return_temp($api_data, $date);
    &print_temp($temp_ref, $date);

    return 1;
}

上記は自分が書いたコード。
コマンドライン引数で日付を取得して、サブルーチン内で日付に関する必要な処理をしている。


my $date = undef;
if ( $ARGV[0] ) {
    # 引数がある場合
    if ( $ARGV[0] =~ /^[0-9]{4}-[0-9]{2}-[0-9]{2}$/ ) {
        $date = $ARGV[0];
    }
    else {
        die "ERROR : argv must be in 'yyyy-mm-dd' format\n";
    }
}
else {
    # 引数がない場合は翌日の日付
    my $dt = DateTime->now( time_zone => 'local' );
    $dt->add( days => 1 );
    $date = $dt->ymd('-');
}

上記は他の人が書いたコード。
my $dateにコマンドライン引数で取得した日付をいきなり代入するのではなく、undefを代入している。
サブルーチン外で日付に関する必要な処理をしているので、サブルーチン内に書くコードが少なくなり見やすくなる。



【まとめ】
・サブルーチンに渡す引数の処理は、そのサブルーチン内ではなくサブルーチンに引数を渡す前に済ませておく。
・「もし〇〇が偽ならば」という書き方はなるべく避ける。




自分が書いたコード。

my $api_key = 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxx';
my $base_url = 'http://api.openweathermap.org/data/2.5/forecast';

my $ua = LWP::UserAgent->new;
my $res = $ua->get(
            "$base_url?q=tokyo,jp&appid=$api_key&units=metric" # http://api.openweathermap.org/data/2.5/forecast?q=tokyo,jp&appid=xxxxxxxxxxxxxxxxxxxxxxx&units=metric
            );


他の人が書いたコード。

sub weather_api {
    # クエリパラメータ
    my $end_point = 'forecast';
    my $city_id   = 1850147;
    my $api_key   = 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxx';
    my $units     = 'metric';
    my $mode      = 'json';

    ## http://api.openweathermap.org/data/2.5/forecast?id=1850147&APPID=xxxxxxxxxxxxxxxxxxxxxxxxxxxxx&units=metric&mode=json 
    my $url        = sprintf( 'http://api.openweathermap.org/data/2.5/%s?id=%s&APPID=%s&units=%s&mode=%s', $end_point, $city_id, $api_key, $units, $mode );
    my $ua         = LWP::UserAgent->new();
    my $api_result = $ua->get($url);
    return $api_result->{_content};
}

クエリパラメータ単位で変数を作って代入している点、
元のURLにパラメーターを指定するにあたってsprintfを使っている点、
$ua->getの引数を変数にしている点が、
自分が書いたコードと特に異なる部分。



【まとめ】
クエリパラメータを使用する際は、クエリパラメータ単位で変数を作ろう。
文字列の途中に変数を入れてかつ展開させる必要がある際は、必ずprintfかsprintfを使おう。