保守性・管理性が上がるPHPの書き方とは?

昨日、PHPの保守性・管理性が上がるPHPの書き方というブログ記事の話題がTLでありました。

どうも炎上して消されてしまったようですが、魚拓があるようです。
http://megalodon.jp/2014-0812-0928-28/bulkserver.jp/blog/2014/08/07/php-code/

あんまりよくない書き方が書かれていて、PHPを随分書いていない俺も卒倒してしまったので、ちょっと俺なりに書いてみようかなと思った次第。

保守性・管理性とは?

そもそも保守性・管理性とは何かと言えば、読みやすさ、理解しやすさ、思い出しやすさだと思います。先の記事がなんで話題になったのかというと、多分、保守性も管理性も乏しかったからです。「これはこう書けるよ」くらいの記事だったら、コーディングを少し減らせるとかでよかったのかもしれません(それでも話題にはなってたかもしれないけれど)。

こういう話は突き詰めていくと、リファクタリングのし易さだの、ユニットテストがあるかだの、フレームワークだの、CMSだの、そういう話になっていくのかもしれません。それが保守性・管理性を考えた上での先人たちの知恵の結晶だからです。しかし、それは後々の話題だと思います。

みんな、いいコード、綺麗なコードが書きたい

綺麗なコードってなんでしょうか?短くてもわかりにくいコードであれば、いいコードとは言えませんし、わかり易くても長ったらしいメソッドや重複があるコードはいいコードとは言いがたいでしょう。だからこそ、スマートな書き方を知りたいと思うのです。その向上心は素晴らしいと思いますので、大事にしていきましょう。

いいコードとは、何をしているのか、意味が伝わるものです。なので、短ければいいというものではありません。これはどの言語でも同じようなものだと思います。

PHPでの綺麗な書き方を考えてみる

いいコードとは、簡潔であること。意味がわかること。変更に強いことです。
ここで、Personクラスと、Pcクラスの2つのクラスを作ってみようと思います。

まずはPersonクラス。

<?php
class Person {
    private $name;
    private $age;
    private $gender;

    function __construct($name, $age, $gender) {
        $this->name = $name;
        $this->age = $age;
        $this->gender = $gender;
    }

    function hello() {
        print "こんにちは、私の名前は{$this->name}です。年齢は{$this->age}歳の{$this->gender}です。\n";
    }

    function isMan() {
        return $this->gender === '男';
    }

    function isWoman() {
        return $this->gender === '女';
    }

    function toJson() {
        return json_encode(get_object_vars($this));
    }
}

次に、Pcクラス。

<?php
class Pc {
    private $cpu;
    private $memory;
    private $ssd;

    function __construct($cpu, $memory, $ssd) {
        $this->cpu = $cpu;
        $this->memory = $memory;
        $this->ssd = $ssd;
    }

    function showSpec() {
        print "cpu: {$this->cpu}\n";
        print "memory: {$this->memory}\n";
        print "ssd: {$this->ssd}\n";
    }

    function toJson() {
        return json_encode(get_object_vars($this));
    }
}

この2つのクラスは、人間とPCなので全く別の性質のクラスですね。

traitを使ってみよう(PHP5.4以上)

PersonクラスとPcクラスでは、同じ定義のメソッドがありました。toJsonメソッドです。
やってることも全く同じで、インスタンスがもつメンバ変数をjsonに変換するというものです。
よくあるサンプルだと、Animalという抽象クラスを作って、それをPersonクラスで継承、Dogクラスで継承して、共通処理はAnimalクラスで定義しておくというのがありますが、これは動物を継承したクラスだからイメージに合うわけですが、PcクラスでAnimalクラスを継承するのはおかしいですよね?でも、クラスの性質が違っても同じようなメソッドを定義することはよくあります。コピペすればすぐできるじゃん!で済まさず、traitにしてみましょう。

traitとは?

traitとは、実装の一部を共通化するための仕組みです。今回の例でいえば、toJsonメソッドのような共通で使えるメソッドを抽出して、各クラスで簡単に実装させることができます。Rubyではmoduleが同じような機能になります。

では、ToJsonImplトレイトを定義してみます。

trait ToJsonImpl {
    function toJson() {
        return json_encode(get_object_vars($this));
    }
}

これを、Personクラスで利用してみましょう。traitを使う場合は、useと書きます。

class Person {
    use ToJsonImpl;
    private $name;
    private $age;
    private $gender;

    // 省略

//    ToJsonImpleトレイトで定義されたので削除する
//    function toJson() {
//        return json_encode(get_object_vars($this));
//    }
}

次に、Pcクラスでも使ってみます。

class Pc {
    use ToJsonImpl;
    private $cpu;
    private $memory;
    private $ssd;

    // 省略

//    ToJsonImpleトレイトで定義されたので削除する
//    function toJson() {
//        return json_encode(get_object_vars($this));
//    }
}

これで、同じようなメソッドを毎回定義しなくてもよくなりました。
同じように、CSVで出力したいんだー!という声があったら、ToCsvImplトレイトを定義すれば、use ToCsvImplと書くだけでPersonでもPcでも、またこれ以降にできた新しいクラスでも使うことができます。便利ですね。


オーバーロードを使ってみよう

次に、__setメソッド, __getメソッドでのオーバーロードを使ってみようと思います。
これは、存在しないメンバ変数にアクセスされた場合の挙動を設定することができます。

Pcクラスでは、CPUとメモリとSSDだけをメンバ変数に持っていましたが、PCってもっと多くのパーツで作られていますよね?グラフィックボードがあったり、CPUファンがあったり、さらに外付けHDDがあったりなど、数え上げると結構な数になります。これらの項目を全て網羅してメンバ変数を作るのは大変です。

class Pc {
    use ToJsonImpl;
    private $cpu;
    private $memory;
    private $ssd;
    private $gpu;
    private $cpu_fan;
    private $display_port1;
    private $display_port2;
    private $display_port3;
    private $display_port4;
    // 書ききれません

    // 省略
}

こういうとき、オーバーロードを使います。オーバーロードを使うと非常にシンプルに書く事ができます。

class Pc {
    use ToJsonImpl;
    private $properties = array();
//    もう使わないので後で削除する
//    private $cpu;
//    private $memory;
//    private $ssd;

    function __construct($cpu, $memory, $ssd) {
        $this->cpu = $cpu;
        $this->memory = $memory;
        $this->ssd = $ssd;
    }

    function __get($name) {
        return $this->properties[$name];
    }

    function __set($name, $value) {
        $this->properties[$name] = $value;
    }

    function showSpec() {
        print "cpu: {$this->cpu}\n";
        print "memory: {$this->memory}\n";
        print "ssd: {$this->ssd}\n";
    }
}

$this->cpu = $cpu; が呼ばれると、__set($name, $value)が呼ばれます。
$nameには”cpu”が、$valueには$cpuに代入されている値が入っています。
結果、$this->properties[“cpu”] = $cpu;というふうになり、properties配列の中にいくらでも値を保存できるようになりました。

そして、$this->cpuを呼ぶと、今度は__get($name)が呼ばれます。
$nameには”cpu”が入っていますから、return $this->properties[“cpu”];というふうになり、properties配列の好きな要素にアクセスできるようになりました。
数えきれないほどのメンバ変数を作るよりは見た目も綺麗だし、新しい要素が増えても、コードの修正は特にありません。

計画性のないオーバーロードは危険

しかしこのままだと、$this->propertiesはなんでもはいる状態です。

$this->rice = "コシヒカリ";

と書いても、入ってしまいます。Pcにriceというメンバ変数があってはダメでしょう…。
なので、制限を入れようと思います。

class Pc {
    use ToJsonImpl;
    private static $PROPERTY_NAMES = array('cpu', 'memory', 'ssd');
    private $properties = array();

    function __construct($cpu, $memory, $ssd) {
        $this->cpu = $cpu;
        $this->memory = $memory;
        $this->ssd = $ssd;
    }

    function __get($name) {
        return $this->properties[$name];
    }

    function __set($name, $value) {
        // PROPERTY_NAMESにある項目しか許可しない
        if (in_array($name, self::$PROPERTY_NAMES)) {
            $this->properties[$name] = $value;
        }
    }

    function showSpec() {
        print "cpu: {$this->cpu}\n";
        print "memory: {$this->memory}\n";
        print "ssd: {$this->ssd}\n";
    }
}

PROPERTY_NAMESに含まれている項目のみ、許可するようにしてみました。
これで、$this->rice = “コシヒカリ”;と書いても無視されます。
変な要素が入れられそうなら、例外を発生させることも、__setメソッド内でできるので、ケースによってはそういう実装もありだと思います。

こうすることで、保守対象はPROPERTY_NAMESのみになります。楽ですねぇ。


メタプログラミングしてみよう。

さて、あとはPcクラスで気になるのは、showSpecメソッドです。

class Pc {
    // 省略

    function showSpec() {
        print "cpu: {$this->cpu}\n";
        print "memory: {$this->memory}\n";
        print "ssd: {$this->ssd}\n";
    }
}

Pcクラスのメンバ変数はオーバーロードによっていくつになるかわからなくなりました。PROPERTY_NAMESによって制限は入れているものの、じゃあgpuを追加したら、showSpecメソッドにgpuの項目を出力するprint文を書くか?と言われたら辛いですね。
そこで、メタプログラミングをしてみましょう。

メタプログラミングとは、メタデータを使ったプログラミングです。ここでは、クラスに付随した情報を元に、showSpecメソッドを書き換えてみます。

class Pc {
    // 省略

    function showSpec() {
        $this->showDetail(get_object_vars($this));
    }

    private function showDetail($properties, $depth=0) {
        $printTab = function($depth){
            for($i = 0; $i < $depth; $i++) {
                print "\t";
            }
        };
        foreach($properties as $key => $value) {
            if (is_array($value)) {
                $printTab($depth);
                print "{$key}:\n";
                $this->showDetail($value, ++$depth);
            } else {
                $printTab($depth);
                print "{$key}: {$value}\n";
            }
        }
    }
}

get_object_vars($this)によって、自身のメンバ変数とその値を配列で取得します。
ここがメタプログラミングです。
とはいってもPcクラスのメンバ変数はpropertiesのみになったので、propertiesとその値の配列が返ります。
privateメソッドのshowDetailメソッド内で、再帰的に呼び出しています。
無名関数を使って、タブを出力するものも作ってみました($printTab)。このメソッド内でしか使わない関数は、使い捨ての関数なのでこのように定義すると、他のところで利用されないため、他のプログラムに影響しなくなります。節度を守って使いましょう。

これを使ってみましょう。

$pc = new Pc("Celeron", "4GB", "128GB");
$pc->showSpec();

結果は以下のようになります。

properties:
	cpu: Celeron
	memory: 4G
	ssd: 128GB

ここで、Pcクラスにhddというパラメーターを追加したいと思います。hddは複数持つ事があるので、配列とします。

class Pc {
    use ToJsonImpl;

    // hddを追加
    private static $PROPERTY_NAMES = ['cpu', 'memory', 'ssd', 'hdd'];
    private $properties = array();

    function __construct($cpu, $memory, $ssd, $hdd=array()) {
        $this->cpu = $cpu;
        $this->memory = $memory;
        $this->ssd = $ssd;
        $this->hdd = $hdd;
    }
}

では、また同じようにやってみましょう。

$pc = new Pc("Celeron", "4GB", "128GB", array("1TB", "1.5TB", "500GB"));
$pc->showSpec();

結果は以下のようになります。

properties:
	cpu: Celeron
	memory: 4G
	ssd: 128GB
	hdd:
		0: 1TB
		1: 1.5TB
		2: 500GB

以上です。
traitやオーバーロード、メタプログラミング、再帰処理は、うまく活用すれば保守性・管理性も上がります。しかし、これらを無理矢理使ってやろうと固執してはいけません。保守しやすいかどうかは、そのコードに関わっている人のレベルにもよります。奇をてらったコーディングはしないようにしましょう。


ransackでデフォルトのソート順を指定する方法

ransackのソートがむちゃくちゃ簡単で便利なことに気付いたのですが、これのデフォルトのソートってどうやるんだろう?と思って調べてみました。
@q.sortsが空だったら指定するだけでした。

class HogesController < ApplicationController
  def index
    search_by_ransack
  end

  private
  def search_by_ransack
    @q = Hoge.search(params[:q])
    @q.sorts = 'id asc' if @q.sorts.empty?
    params[:page] ||= 1
    @hoges = @q.result.page(params[:page]).per(10)
  end
end

ransackでidをカンマ区切りで指定する方法

ransack便利ですよね。検索作るときはもうこれなしでは考えられません。
ところで、idを複数指定したい場合はどうするんだろうかな?と思って調べていたんですが、どうも複数の入力エリアを作ってそこに個別に入れていくという感じっぽかったのですが、これ面倒だなーと思ったので、カンマ区切りでid指定できるようにしてみました。

まずはView側から。複数IDの指定での抽出がやりたいので、_inを使います。
肝なのは、text_fieldへのvalueの展開ですが、配列をカンマ区切りで出力しています。

= search_form_for @q do |f|
  = f.label :id_in, 'ID'
  = f.text_field :id_in, {value: (params[:q].present? && params[:q][:id_in].present?) ? params[:q][:id_in].join(',') : ''}
  = f.submit 'Search'

次は、Controller側です。
params[:q][:id_in]の値はただの文字列なので、配列に変換しています。
ransackのソートを使っている場合、params[:q][:id_in]の値がまんま配列として渡ってくるので、配列である場合は何も処理しないようにしています(unlessの部分)。

class HogesController < ApplicationController
  def index
    search_by_ransack
  end

  private
  def search_by_ransack
    if params[:q].present? && params[:q][:id_in].present?
      unless params[:q][:id_in].is_a? Array
        params[:q][:id_in] = params[:q][:id_in].split(',').map{|v| v.strip}
      end
    end
    @q = Hoge.search(params[:q])
    params[:page] ||= 1
    @hoges = @q.result.page(params[:page]).per(10)
  end
end

もっとスマートな方法があるかもしれませんが、やりたいことできたのでこれでOKとします。


Devise(Warden)でログインしたときにセッションIDを変えさせない方法

これはとくまるひろしのSession Fixation攻撃入門でも紹介されている通り、セッションハイジャックの可能性を高めてしまうので、本来はオススメではありません。私も違う方法を取りました。

しかし、調べるのが大変だったので、備忘録として残すこととします。
環境は、

  • Rails 4.1.4
  • Devise

作っていたものは、以下のようなもの。

  1. 既にアカウントはログイン済みである。
  2. 他の場所でアカウントのログインが試みられる
  3. 一旦ログインさせずに、他のユーザーがログインしていることを通知
  4. それでもログインしたかったら現在ログイン中のユーザーをログアウトさせてログインする

これを実現させるために、ログインしたタイミングでセッションIDを保存しておいて、現在のセッションIDと違う場合は弾くというふうにしようと思い、作っていました。ところが、DeviseというかWardenは、ログインしたタイミングでセッションIDが変わるみたいでした。そのため、保存したセッションIDは既に無効になっていました。

これは、先のリンクにある、Session Fixation Attackへの防御の為のようです。
参考リンク:https://github.com/hassox/warden/issues/94

とりあえずの回避策としては、セッションIDが変わらなければいいので、まずはそれを目指しました。できたコードがこれ。

class User::SessionsController < Devise::SessionsController
  Warden::Manager.after_authentication do |user, auth, opts|
    auth.request.session_options[:renew] = false
  end

  # 後はcreateとかでログイン処理したタイミングでセッションIDを保存する
end

こうすると、ログイン前とログイン後でセッションIDが変わりません。

わかるとたいした事はないのですが、ここに行き着くのに1日費やしてしまいました。そもそもが、あんまり需要のない機能なのかもしれません…。とりあえず解決はしたのですが、これだと先の攻撃をされる可能性があり得るので、違う方法で回避したいと思い、次のようにしました。

最終的な回避策としては、セッションID更新予約フラグをセッションに持たせました。

class User::SessionsController < Devise::SessionsController
  def create
    self.resource = warden.authenticate!(auth_options)
    if resource.other_already_signed_in?(request.session_options[:id])
      warden.logout
      session[:user_id] = resource.id
      redirect_to user_confirmation_login_path
    else
      super do |resource|
        set_reserve_update_session_id
      end
    end
  end

  def confirmation_login
  end

  def force_login
    self.resource = User.find session[:user_id]
    set_flash_message(:notice, :signed_in) if is_flashing_format?
    sign_in(resource, event: :authentication)
    set_reserve_update_session_id
    respond_with resource, location: after_sign_in_path_for(resource)
  end

  private
  def set_reserve_update_session_id
    session[:reserve_update_session_id] = true
  end
end

これを、application_controller.rbで確認させるようにしました。
強制ログアウトもあるので、prepend_before_actionを使って、ブロックで処理しました。

class ApplicationController < ActionController::Base
  prepend_before_action do
    update_session_id
    check_force_logout unless devise_controller?
  end

  private
  def update_session_id
    if current_user && session[:reserve_update_session_id]
      current_user.session_id = request.session_options[:id]
      current_user.last_access_at = Time.now
      current_user.save!
      session.delete :reserve_update_session_id
    end
  end

  def check_force_logout
    if current_user && current_user.session_id != request.session_options[:id]
      sign_out current_user
      redirect_to new_user_session_path, alert: '他の方がログイン中のため、ログアウトされました'
    end
  end
end

これでようやく前に進めるー!!


rack-dev-markで好きな文字列を表示する方法

Qiita: Rails4 今のところ最強なデバッグツール達を読んでいたら、rack-dev-markというgemがコメントで紹介されてました。このgemは、開発中の環境であったり、ステージングの環境であったら、リボンを表示することができます。
現在開発中のWebアプリケーションが、いろんなところにデプロイするやつなので、こういうのを丁度欲していました。

https://github.com/dtaniwaki/rack-dev-mark

しかし、デフォルトだと

  • 表示する文字列がRAILS_ENVに依存する。
  • Herokuのステージングなどの場合は、環境変数 RACK_DEV_MARK に好きな値を入れられる

という感じなのですが、いろんな環境があるので、もっと自由に表示できたらなーと思い、以下のようにしてみました。

  1. #{Rails.root}/.rack_dev_markというファイルを作る。そのファイルに表示する文字列を書いておく。
  2. config/application.rbに以下を追加する。
    module MyApp
      class Application < Rails::Application
        config.rack_dev_mark.enable = File.exist?("#{Rails.root}/.rack_dev_mark") || ENV['RACK_DEV_MARK']
      end
    end
    
  3. config/initializers以下に、rack_dev_mark.rbというファイルを作って以下のように記述する。
    if File.exist? "#{Rails.root}/.rack_dev_mark"
      Rack::DevMark.env = File.read "#{Rails.root}/.rack_dev_mark"
    end
    
  4. .rack_dev_markファイルはgitで管理したくないので、.gitignoreに追加しておくこと。

こうすることで、.rack_dev_markファイルがあったら自動的にリボンが表示されるようになりました。いろんな環境にデプロイしないといけない場合には、目印になっていいと思います。