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

・上記APIで東京の天気を取得して、翌日の最高気温、最低気温を摂氏で表示する
・日付はコマンドライン引数でも指定できるようにする(指定なしの場合は翌日)
・APIのレスポンスはJSON形式で受け取り、そのJSONと日付指定をもとに最高気温と最低気温を摂氏で返却する処理はサブルーチン化して、そのサブルーチンのテストコードを書く
　引数: JSON文字列、日付('2018-04-11'などの文字列)
　戻り値: 最高気温と最低気温が入ったハッシュリファレンス / 取得できなければundef

上記課題を解くにあたって、どのような構成でコードを書けば見やすくなるか。

■自分が書いたコードの構成

&get_date_temp($date);　# 東京の天気を取得して、翌日の最高気温、最低気温を摂氏で表示するサブルーチンの呼び出し。

sub get_date_temp{
   日付の処理。
    my $api_data = &get_api_data;　#APIからJSONデータを取得して$api_dataに代入する。
    my $temp_ref = &return_temp($api_data, $date);　#12時の気温、最高気温、最低気温の情報が入っているハッシュのリファレンスを$temp_refに代入する。
    &print_temp($temp_ref, $date);　#$temp_refをprintする。
}

#get_api_dataの定義。
sub get_api_data{
...
}

#return_tempの定義。
sub return_temp{
...
}

#print_tempの定義。
sub print_temp{
...
}

■他の人が書いたコードの構成

日付の処理。

my $result = &weather_api;　#APIからJSONデータを取得して$resultに代入する。

my $temp_noon = &get_temp_at_noon( $result, $date );　# 12時の気温を取得してprint。
print ...

my $temp_maxmin = &get_temp_maxmin( $result, $date );　# 最高気温と最低気温を取得してprint。
print...

#weather_apiの定義。
sub weather_api {
...
}

#get_temp_at_noonの定義。
sub get_temp_at_noon {
...
}

#get_temp_maxminの定義。
sub get_temp_maxmin {
...
}

日付についての処理をサブルーチン外で行っている点、
printするにあたってサブルーチンを使っていない点、
最高気温と最低気温の取得と、12時の気温の取得を行うサブルーチンを分けている点が、
自分のコードと異なる。

個人的には、全体の作業を1つのサブルーチンにまとめてそのサブルーチン内で個々のサブルーチンを呼び出すスタイルの方が好き。
ただ、先輩が書いたコードを見ている限りそのようなスタイルの書き方は見かけないので避けた方がいいのかもしれない。

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

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

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を使おう。

2018-05-25

OpenWeatherMapのAPIで東京の天気を取得して、翌日の最高気温、最低気温を摂氏で表示する。

■メインコード

package Indicate_TEMP;

use Exporter 'import';
our @EXPORT = qw/get_date_temp get_api_data return_temp print_temp/;
our $VERSION = "0.0.1";

use LWP::UserAgent;
use strict;
use warnings;
use utf8;
use Encode;
use JSON::XS 'decode_json';
use Data::Dumper;
use DateTime;



#API Key、URLの代入。
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;
}




#apiからのデータの取得処理を行うサブルーチンを作成。
sub get_api_data{
    my $api_key = 'ed0e5aec8d17204480f7bde57279e1af';
    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=ed0e5aec8d17204480f7bde57279e1af&units=metric
            );

    my $file = "{}";
    if ($res->is_success) {
        $file = $res->content;
    } else {
        print "Failed to acquire web data.\n";
        return undef;
    }


    my $api_data = decode_json($file);
    return $api_data;
}


#指定された日の12時の気温、最高気温、最低気温が入ったハッシュリファレンスを返すためのサブルーチンを作成。
sub return_temp{
    my ($api_data, $date) = @_;
    unless (defined($api_data)){
        print "API data is not defined.\n";
        return undef;
    }
    my $date_12pm = $date." 12:00:00";
    my $key;


#12時の気温の取得。
    my $temp_12 = 'undef';
    foreach $key(keys $api_data->{'list'}){
        my $dt_txt_phr = $api_data->{'list'}->[$key]->{'dt_txt'};
        if ($dt_txt_phr) {
            if ($dt_txt_phr =~ /$date_12pm/){
                $temp_12 = $api_data->{'list'}->[$key]->{'main'}->{'temp'};
            }
        }
    }

#最高気温と最低気温の取得。
    my $max;
    my $min;
    my $temp_max;
    my $temp_min;
    foreach $key (keys $api_data->{'list'}) {
        my $dt_txt_phr = $api_data->{'list'}->[$key]->{'dt_txt'};

        if ($dt_txt_phr) {
            if ($dt_txt_phr =~ /$date/){
                $temp_max = $api_data->{'list'}->[$key]->{'main'}->{'temp_max'};
                $temp_min = $api_data->{'list'}->[$key]->{'main'}->{'temp_min'};

                unless (defined($max)){
                    $max = $temp_max;
                }
                unless (defined($min)){
                    $min = $temp_min;
                }

                if ($max < $temp_max) {
                    $max = $temp_max;
                }
                if ($min > $temp_min) {
                    $min = $temp_min;
                }
            }
        }
    }

    unless (defined($max)) {
        $max = 'undef';
    }
    unless (defined($min)) {
        $min = 'undef';
    }

    my %temp = ("12pm_temp" => "$temp_12", "temp_max" => "$max", "temp_min" => "$min") ;
#print %temp;
    if (%temp){
        return \%temp;
    } else {
        return undef;
    }
}


#指定された日の12時の気温、最高気温、最低気温をprintするためのサブルーチンを作成。
sub print_temp{
    my ($temp_ref, $date) = @_;
    my $str_1 = sprintf (qq/%sの12:00の気温は%s°cです。\n/, $date, $temp_ref->{"12pm_temp"});
    my $str_2 = sprintf (qq/%sの最高気温は%s°cです。\n/, $date, $temp_ref->{"temp_max"});
    my $str_3 = sprintf (qq/%sの最低気温は%s°cです。\n/, $date, $temp_ref->{"temp_min"});

    my $enc_str_1 = encode('utf-8', $str_1);
    my $enc_str_2 = encode('utf-8', $str_2);
    my $enc_str_3 = encode('utf-8', $str_3);

    print $enc_str_1;
    print $enc_str_2;
    print $enc_str_3;
}




1;

■テストコード

use strict;
use warnings;
use JSON::XS 'decode_json';
use Test::More;

BEGIN {
    use_ok('Indicate_TEMP') || BAIL_OUT();
}

diag("Testing Indicate_TEMP $Indicate_TEMP::VERSION, Perl $], $^X");



# サブルーチンが定義されているかのテスト。
ok( defined &Indicate_TEMP::get_date_temp, 'Indicate_TEMP::get_date_temp is defined' );

ok( defined &Indicate_TEMP::get_api_data, 'Indicate_TEMP::get_api_data is defined' );
ok( defined &Indicate_TEMP::return_temp,  'Indicate_TEMP::return_temp is defined' );
ok( defined &Indicate_TEMP::print_temp,   'Indicate_TEMP::print_temp is defined' );




{
    use Indicate_TEMP;

#関数get_date_tempのテスト。
    {
        my $api_data = {'list' => [
            {
                'dt_txt' => '2018-05-24 12:00:00',
                    'main' => {
                        'temp_max' => '18.74',
                        'temp' => '18.74',
                        'temp_min' => '17.61',
                    }
            }
        ]
        };
        my $date = "2018-05-24";
        is( get_date_temp($date), 1);

        $date = "22222222222222";
        is( get_date_temp($date), undef);

        $date = ();
        is( get_date_temp($date), 1);
    }


#関数get_api_dataのテスト
    {
        my $api_data = {'list' => [
            {
                'dt_txt' => '2018-05-24 12:00:00',
                    'main' => {
                        'temp_max' => '18.74',
                        'temp' => '18.74',
                        'temp_min' => '17.61',

                    }
            }
        ]
        };
        ok( get_api_data, $api_data);

        my $file = "";
        ok( get_api_data, undef);

        my $api_key = "";
        ok( get_api_data, undef);

        my $base_url = "";
        ok( get_api_data, undef);

    }

#関数return_tempのテスト
    {
        my $api_data = {'list' => [
            {
                'dt_txt' => '2018-05-24 12:00:00',
                    'main' => {
                        'temp_max' => '18.74',
                        'temp' => '18.74',
                        'temp_min' => '17.61',

                    }
            }
        ]
        };
        my $date = "2018-05-24";

        $api_data = undef;
        is( return_temp($api_data, $date), undef);

    }
}


done_testing;

2018-05-24

①gitにおいて、コミットしたファイルのコミットメッセージを修正したり、コミットの削除をする時の注意点

On branch kadai4_review
Unmerged paths:
  (use "git reset HEAD <file>..." to unstage)
  (use "git add/rm <file>..." as appropriate to mark resolution)

	deleted by them: t/Indicate_TEMP.t

Untracked files:
  (use "git add <file>..." to include in what will be committed)

	.Indicate_TEMP.pm.swo
	.gitignore~
	kadai4-1.pl
	kadai4.pl
	t/test.pm

no changes added to commit (use "git add" and/or "git commit -a")

たとえ削除されていたとしても、上記のようにmergeされていないファイルがある状態で、

git commit --amend

git revert

上記のコマンドを実行すると

error: Reverting is not possible because you have unmerged files.
hint: Fix them up in the work tree, and then use 'git add/rm <file>'
hint: as appropriate to mark resolution and make a commit.
fatal: revert failed

というエラーが返ってくる。

git reset --hard HEAD^

上記のコマンドを実行して、Indicate_TEMP.tのコミットの取り消しを行うとエラーが消えた。

参考
git push の取り消し方法。 | WWWクリエイターズ
[Git]コミットの取り消し、打ち消し、上書き - Qiita
Git初心者に捧ぐ！Gitの「これなんで？」を解説します。 | KRAY Inc

②下記コマンドの実行ができない時の対処。

$ cpanm --sudo install Catalyst::Devel
$ catalyst.pl MyApp

$ sudo yum install cpan
$ sudo cpan App::cpanminus

上記コマンドで解決。
前の課題の時から何度もcpanを使っていたのに、インストール済みと表示されなかった原因は何？？

■その他所感
・テストコードについて
└入力パターンに対して期待する出力パターンが得られるか。
　エラーを期待する場合はエラーが表示されて終了するか。
　これはある1パターンではなく、考えうる複数のパターンについてそうなるか。
└メインコード内のすべて分岐を通るようなテストコードを書くのが理想。

・ソースコードの見た目について。
└他の人が読みやすいコードを書く。
参考
ソースコードを美しくデザインする - Qiita
プログラミング作法 - Wikipedia

【Perl】サブルーチンの呼び出しについて

package Indicate_TEMP;

use Exporter 'import';
our @EXPORT = qw/get_date_temp get_api_data return_temp print_temp/;
our $VERSION = "0.0.1";

use LWP::UserAgent;
use strict;
use warnings;
use utf8;
use Encode;
use JSON::XS 'decode_json';
use Data::Dumper;
use DateTime;



#API Key、URLの代入。
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;
}

サブルーチンの中でサブルーチンを呼び出すのは可能だが、サブルーチンの中でサブルーチンを定義するとコードが読みづらくなるので、避けましょうというお話。

use utf8;を使用して"Wide character in print at〜"というエラー文が出た時の対処について

下記サブルーチンを実行したところ、"Wide character in print at〜"というメッセージが表示された。

&print_temp($temp_ref, $date);
sub print_temp{
    my ($temp_ref, $date) = @_;
    printf (qq/%sの12:00の気温は%s°cです。\n/, $date, $temp_ref->{"12pm_temp"});
    printf (qq/%sの最高気温は%s°cです。\n/, $date, $temp_ref->{"temp_max"});
    printf (qq/%sの最低気温は%s°cです。\n/, $date, $temp_ref->{"temp_min"});
}

エンコーディングしていないのが問題だと判明。 

参考：Perl の文字列エンコーディングの話 | Hachioji.pm 日めくりテックトーク

先ほどのコードの書き方だと、内部表現をそのまま出力しようとしていたようで、下記のようにエンコードしたところエラーが消えた。

&print_temp($temp_ref, $date);
sub print_temp{
my ($temp_ref, $date) = @_;
my $str_1 = qq/$date/.qq/の12:00の気温は$temp_ref->{"12pm_temp"}°cです。\n/;
my $str_2 = qq/$date/.qq/の最高気温は$temp_ref->{"temp_max"}°cです。\n/;
my $str_3 = qq/$date/.qq/の最低気温は$temp_ref->{"temp_min"}°cです。\n/;

my $enc_str_1 = encode('utf-8', $str_1);
my $enc_str_2 = encode('utf-8', $str_2);
my $enc_str_3 = encode('utf-8', $str_3);

print $enc_str_1;
print $enc_str_2;
print $enc_str_3;
}