perl - 勝手に添削 - HTTP::MobileUserID::Japanese

Perl Best Practices
Damian Conway
[邦訳:Perlベストプラクティス]

今回は、こちら。

• Unknown::Programming - 携帯のユーザーIDを取得する

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

# ...

sub is_support     {  shift->{support} }
sub is_not_support { !shift->{support} }

sub is_exists     {  shift->{user_id} }
sub is_not_exists { !shift->{user_id} }

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

is_exists()の方は、has_user_id()が望ましいだろう。

ついでに、$self->{support}も$self->{supported}に直し、__PACKAGE__->mk_classaccessors()の引数にsupportedも追加すれば、is_support()相当のメソッドは定義不要になる。

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

全部まとめると、以下のとおりか。

package HTTP::MobileUserID::Japanese;

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

our $VERSION = '0.01_01'; # or 0.02

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

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

*id = *has_user_id = \&user_id;

sub init {
    my $self = shift;
    $self->agent(shift);    # Agentはここで設定
    if ( $agent->is_docomo ) {
        # $agent->html_version は存在しなければ 0
        $self->supported(0) if $agent->html_version <= 2.0;
        $self->user_id(
            $self->supported ? $agent->serial_number : undef );
    }
    elsif ( $agent->is_softbank ) {
        $self->supported(0) if $agent->is_type_c;
        $self->user_id(
            $agent->get_header('x-jphone-uid') ? $user_id : undef );
    }
    elsif ( $agent->is_ezweb ) {
        $self->user_id( $agent->get_header('x-up-subno') );
    }
    else {
        $self->supported(0);
    }
    $self;
}

1;

Dan the Perl Monger

Comments

is_suppoted → is_supported
ですね。

Posted by imsut at 2007年05月06日 17:18