勝手に添削を添削 HTTP::MobileUserID

Perl

うぉぅ。弾さんから初の添削頂きました。

#あ、ちなみにパッケージ名だけどHTTP::MobileAgentと揃えるためにJapaneseは無しにしました

コードそのものというより、メソッド名がまずい。is_support()では英語になっていない。日本専用モジュールではあるけど、CPANとかにうpするとしたら、これだとキョーレツに違和感がある。正しくはis_suppoted()またはis抜きのsupported()。 is_exists()の方は、has_user_id()が望ましいだろう。

きゃああ。これは恥ずかしい。

なんでもかんでもis_をつければそれっぽいっしょと考えてることがバレバレですねorz

こういうメソッドの命名ってうまくできないなぁ。色々なソース見たりして都度勉強していくしかないですかね。

こういうときはis_〜、こういうときはhas_〜、またこういうときは〜edみたいな指標があればいいのにね。まあ他人任せにするのではなく自分で切り開いていくもんかもしれないけども。

で次

それから、Accessor(正確にはMutator)がすでに存在する場合は、初期化ルーチンでも$self->{key} = valueとするより$self->key(value)の方が後の事を考えると吉。

これは完全に僕のミス。そうでした・・・。特にClass::Data::Accessorみたいなのを使ってるときは$self->key(value)を使わないとパッケージ側で値を定義していた場合に消されてしまう。

とまぁこんなもんでしょうか。

弾さん添削の記事を見つけたときにもっと色々なんか言われるのかと思ったけどそんなにクリティカルではなかったのでちょっとほっとしてたり。

ただ命名に突っ込まれるほうが恥ずかしい限りですがorz

で弾さんに添削してもらったソースですが、supported()が初期値に1が入ってないのinitのところとかで$self->supported(1)としなければならないですね。

あとhtml_versionの判定ですが、

 # $agent->html_version は存在しなければ 0
 $self->supported(0) if $agent->html_version <= 2.0;

これはまずいです。

$agent->html_versionが存在しないという状況はつまり新しいDoCoMo端末が出たということです。

で新しい端末ならほぼ間違いなくutnに対応しているはずです。（まさか今更utn非対応端末出すとは考えにくい。もし出たとしたらHTTP::MobileAgent側で更新してもらわないといけません。）

とするとsupported()は真になっていて欲しいわけです。

なのでやはりここは

 $self->supported(0) if $agent->html_version && $agent->html_version <= 2.0;

とすべきかと。

ってことでこれらを踏まえた上で書き直しました。

package HTTP::MobileUserID;

use strict;
use warnings;
use base qw/Class::Data::Accessor/;
__PACKAGE__->mk_classaccessors(qw/agent user_id supported/);

our $VERSION = '0.01';

sub new {
    my $proto = shift;
    my $self = bless {} , ref $proto || $proto;
    $self->init(@_);
    return $self;
}

sub has_user_id {  shift->user_id   }
sub no_user_id  { !shift->user_id   }
sub unsupported { !shift->supported }

*id = \&user_id;

sub init {
    my $self  = shift;
    my $agent = $self->agent(shift);
    $self->supported(1);
    
    if ( $agent->is_docomo ) {
        $self->supported(0) if $agent->html_version && $agent->html_version <= 2.0;
        return if $self->unsupported;
        $self->user_id($agent->serial_number);
    }
    elsif ( $agent->is_softbank ) {
        $self->supported(0) if $agent->is_type_c;
        return if $self->unsupported;
        my $user_id = $agent->get_header('x-jphone-uid') || 'NULL';
        $self->user_id($user_id) if $user_id ne 'NULL';
    }
    elsif ( $agent->is_ezweb ) {
        $self->user_id($agent->get_header('x-up-subno'));
    }
    else {
        $self->supported(0);
    }
}

1;

追記：

HTTP::MobileUserID & Catalyst::Plugin::MobileUserID Released - Unknown::Programming