PHPプログラミング診断室

第3回郷に入っては郷に従え(診断編)

新年あけましておめでとうございます。昨年暮れに始まった本連載ですが、みなさんの現場にあるPHPコードを改善すべく、診断を行ってきたいと思います。本年もよろしくお願いします!

PHPにはPHPの書き方がある

2014年最初の患者さんは、とあるECサイトにて動いていたコードです。ショッピングカートを表現するクラスで、書かれた時期は2000年ごろです。動作していたPHPのバージョンは、PHP 3の国際化版というオールドマニアには垂涎もののバージョンです。当時は、PHPが普及しだしたころで、まだ標準的なPHPコードの書き方というものがない状況でした。そのため、ほかの言語で培った流儀をそのまま持ってきて書かれたコードがよく見られました。

今回は、そんなほかの言語の書き方を持ってきてしまったコードです。さあ、お入りください。

<?php
/*=============================================================================
    title   :   Cartクラス

    commnets:   買い物カゴを定義する
                コンストラクタで、一意のカートID&カートDBに新レコードを
                生成。
                カートIDはクッキーに発行し、セッション管理を行う
    how to  :

    filename:   cart.inc

    date    :   2000/06/22
  ===========================================================================*/

/*---------------------------------------------------------------------------*/
/*  定数定義
/*---------------------------------------------------------------------------*/
define("CART_ISSUE_TIME", 24);    //カート有効時間

class Cart{
    //メンバ変数
    var $a_Item = "";       //アイテム情報
    var $cCartId;           //カートID
    var $iMoney = 0;        //合計金額

    /*-----------------------------------------------------------------------*/
    /*  コンストラクタ
    /*-----------------------------------------------------------------------*/
    function Cart(){
        global $CK_cCartId;

        //カートIDセット
        if(!$CK_cCartId)    $CK_cCartId = make_unique_code("cart", "id", 8);
        $this->cCartId = $CK_cCartId;
    }

    /*-----------------------------------------------------------------------*/
    /*  商品を追加する
    /*-----------------------------------------------------------------------*/
    function do_add_item($cGoodsCode){
        $a_Value[0] = $this->cCartId;
        $a_Value[] = date("YmdHis", mktime(date("H") + CART_ISSUE_TIME, date("i"), date("s"), date("m"), date("d"), date("Y")));    //有効時間時間;
        $a_Value[] = $cGoodsCode;
        $a_Value[] = 1;

        $db = new DB;
        $db->do_insert("cart", "", $a_Value);
    }

    /*-----------------------------------------------------------------------*/
    /*  商品個数を修正する
    /*-----------------------------------------------------------------------*/
    function do_change_item($cGoodsCode, $iGoodsNum){
        $a_Value[0] = $iGoodsNum;

        //一時削除
        $this->do_delete_item($cGoodsCode);

        $a_Value[0] = $this->cCartId;
        $a_Value[] = make_date("YmdHis");
        $a_Value[] = $cGoodsCode;
        $a_Value[] = $iGoodsNum;

        $db = new DB;
        $db->do_insert("cart", "", $a_Value);
    }

    /*-----------------------------------------------------------------------*/
    /*  商品を削除する
    /*-----------------------------------------------------------------------*/
    function do_delete_item($cGoodsCode){
        if($cGoodsCode == -1) $cWhere = " WHERE id=\"" . $this->cCartId . "\"";
        else                  $cWhere = " WHERE id=\"" . $this->cCartId . "\" AND gds_id=\"" . $cGoodsCode . "\"";

        $db = new DB;
        $db->do_delete("cart", $cWhere);
    }

    /*-----------------------------------------------------------------------*/
    /*  カートIDをクッキーに保存
    /*-----------------------------------------------------------------------*/
    function set_cookie(){
        setcookie("CK_cCartId", $this->cCartId, time()+3600);
    }

    /*-----------------------------------------------------------------------*/
    /*  カートを削除
    /*-----------------------------------------------------------------------*/
    function delete_cart(){
        //クッキーを削除
        setcookie("CK_cCartId", $this->cCartId, time()-1);
        //DBを削除
        $db = new DB;
        $db->do_delete("cart", " WHERE id=\"" . $this->cCartId . "\"");
    }

    /*-----------------------------------------------------------------------*/
    /*  メンバ変数参照
    /*-----------------------------------------------------------------------*/
    /*----------    カートID                                        ---------*/
    function get_id(){
        return $this->cCartId;
    }

    /*----------    商品情報                                        ---------*/
    function get_item(){
        return $this->a_Item;
    }

    /*----------    合計個数                                        ---------*/
    function get_count(){
        $count = 0;
        while($this->a_Item[$i]){
            $count += $this->a_Item[$i];
        }
        return $count;
    }
}
?>

診断

1. 文字エンコーディングがShift_JIS、改行コードがCRLF

このPHPコードは、文字エンコーディングがShift_JIS、改行コードがCRLFで記述されていました(この記事ではUTF-8で表記しています⁠⁠。

PHPコードをShift_JISで書くことがただちにダメというわけではないのですが、正常に動作させるには、PHPのコンパイルオプションや設定を調整する必要があります。また、ファイルによって文字エンコーディングが異なると、コードの差分を取る際に、コード自体は同じなのに異なるコードとして検出されるなど、差分を確認することが難しくなります。これは改行コードについても同様です。

現在では、文字エンコーディングをUTF-8で記述することでほぼ不都合はないため、文字エンコーディングはUTF-8、改行コードはLFに統一して記述することが推奨されています。

もし携帯電話など一部の古いデバイス向けにShift_JISを利用せざるを得ない場合は、PHPコード自体はUTF-8で記述しておき、出力時にmb_output_handler()mb_convert_encoding()を使って、UTF-8からShift_JISへ変換して出力するのがよいでしょう。

2. ファイル拡張子が.inc

このコードのファイル名は「cart.inc」です。この「.inc」という拡張子、昨今ではあまり見ないものですが、以前はよく用いられていました。この拡張子が付いたファイルは、外部に公開するものではなく、別のPHPファイルから読み込まれることを想定していました(includeだから.incですね⁠⁠。後述するハンガリアン記法にも少し通じるのですが、拡張子にファイルの動作とは別な意味を持たせていました。

意図はわからないことはないですが、注意しておかないといけないことがあります。もし、このファイルをそのままDocumentRoot以下に置いておくと、外部から直接アクセスされた場合、なんとソースコードがそのまま出力されてしまいます! これは、Webサーバが.incという拡張子のファイルをPHPファイルとして認識していないためで、WebサーバがApacheなら、addHandlerSetHandlerでPHPコードとして実行されるように設定を行う必要があります

また、IDEなどの開発ツールでも.incがPHPファイルであることを認識していないものがあるので、これもまた設定が必要となります。

現在はこうしたPHPファイルの種別によって拡張子を変えるという慣習はなく、PHPコードが書かれたファイルであれば、すべて「.php」という拡張子にします。

3. これはPHP?

コードを見て目に付くのが、特徴的なコメントの書き方です。このコードを書いた方は、C言語やVisual Basicなどの別の言語を書いた経験があるそうで、おそらくその流れでこういったコメントの書き方になっているのでしょう。今のPHPを知っている人であれば、おおよそこういった書き方はしません。

PHPにはPHPDocというコメント形式が広く使われています。PHPDocはJavadocから派生したもので、多くの記法はそれによく似ています。PHPでコードを書くうえではこの形式が一般的なので、PHPDocの形式に従うことで、誰もが理解しやすいコメントを書くことができます。

たとえば、38~40行目にdo_add_item()メソッドのコメントが次のように書かれています。

    /*-----------------------------------------------------------------------*/
    /*  商品を追加する
    /*-----------------------------------------------------------------------*/

これをPHPDoc形式で書くと次のようになります。引数や戻り値についても記載があり、このメソッドのIN/OUTがわかりやすくなっています。

    /**
     * 商品を追加する
     *
     * @param string $cGoodsCode
     * @return void
     */

PHPDocを使う理由は、コードを読む人がわかりやすいということだけではありません。PHPには、リフレクションによるPHPDoc形式のコメントを抽出する機能があり、これを利用してメタデータの読み取りやアノテーションを実現しているツールがあります。たとえば、phpDocumentor では、PHPDoc形式のコメントからAPIリファレンスを自動生成することができます。また、EclipseやPhpStormといったIDEでは、補完候補やヒントの表示にPHPDoc形式のコメントを利用しています。

華麗なアスキーアートによるコメントも味があるのですが、PHPではPHPDoc形式によるコメントが広く使われているので、それに従うのが賢明でしょう。

4. 変数名がハンガリアン

次に気になるのは、変数名です。変数名の先頭には何やら「c」「a_」⁠i」といったプレフィックス(接頭辞)が付いています。おそらくこれは変数の型を示すもので、⁠c」はcharacter型(string型?⁠⁠、⁠a_」はarray型、⁠i」はinteger型を示すものだと推測します。こうした型に関する情報を変数名の前や後ろに付与して命名する記法をハンガリアン記法と呼びます。このハンガリアン記法は、以前のWindows系の開発でよく用いられていたもので、コードが書かれた年代を合わせて考えると、おそらくWindows系の開発をされていた方がこのコードを書いたのではないかと推察できます。

現在ではあまり見なくなったハンガリアン記法ですが、変数名を見ただけでデータ型がわかるという意味では、悪くないように思えます。ハンガリアン記法が良くない点というのはいくつかあるのですが、一番は「みんなが共通に理解しているルールが存在しない」ということが大きいように思います。おそらく前項にあるPHPDocのように標準化がされていれば、もしかするとこうして書くのが当たり前の世の中になっていたのかもしれないのですが、残念ながら今ではそうした記述は行われていません。

おそらくハンガリアン記法を知らない人にとっては、$a_Item変数の「a_」が何を意味するのか、想像が付かないでしょう。これが$items$itemListなどであれば、itemが複数入っているリストのようなものであることが簡単に認識できます。つまり、変数の型を名前に含めるより、その変数が意味するところを明確に書いたほうがわかりやすいということですね。

ちなみに、30行目に登場する$CK_cCartId変数は、いろいろ考えたのですが、わかりませんでした。⁠cCartId」「c」は、characterだと思うのですが、その前の「CK_」は何でしょうね……[1]⁠。うーん、といった具合に意味が取れないのであれば、ハンガリアン記法は役に立たないということです。

5. global文

30行目のコンストラクタで登場するのが悪名高きglobal文です。これを見たら思わず身構えてしまうPHPerの諸兄も多いかと思います。

ご存じない方に向けて解説すると、global文はグローバル変数をローカルスコープ内から操作できるようにする命令です。下記の例では、$CK_cCartIdというグローバル変数を操作できるようにしています。つまり、このコンストラクタ内では、グローバル変数$CK_cCartIdの参照も値の変更も可能になります。

    function Cart(){
        global $CK_cCartId;

        //カートIDセット
        if(!$CK_cCartId)    $CK_cCartId = make_unique_code("cart", "id", 8);
        $this->cCartId = $CK_cCartId;
    }

グローバル変数の利用はできるだけ避けたほうがよいです。

その理由はいくつかあるのですが、まずグローバル変数は、影響範囲が大きいということです。名前のとおりグローバルな変数なので、PHPコードが動作している個所ではどこでも参照、変更が可能です。つまり、仮に値を変更した場合、予期せぬ個所にもその影響が波及してしまう可能性があります。この影響する個所を正確に把握しながらコードを書くというのは、なかなか大変な作業です。影響範囲を絞り、限られたスコープの中で変数の操作を行う方が、理解しやすく、安全なコードを書きやすくなります。

どうしてもグローバル変数を使わないと処理が書けない、もしくは使ったほうが遥かに効率が良いという場面があれば、その用途に限定して使うということはあるかもしれませんが、使う必要がないのに安易にグローバル変数を利用するのは避けるべきです。上記のコードはまさに悪い例で、単にコンストラクタの引数に$CK_cCartIdの値を引き渡すだけで用が済みます。これを実装した例が下記になります。$CK_cCartIdの値は、Cartクラスを利用する側が生成しておき、インスタンス化する際に引数として渡すようにしています。

コンストラクタで$cCartIdを引数にとる
    function Cart ($cCartId){
        $this->cCartId = $cCartId;
    }
$CK_cCartIdをコンストラクタに渡す
if (!$CK_cCartId) {
   $CK_cCartId = make_unique_code("cart", "id", 8);
}
$cart = new Cart($CK_cCartId); // コンストラクタに$cCartIdを渡す

ほかには、カートIDの生成はコンストラクタで行い、Cartクラスのメンバ変数$cCartIdにだけ、その値を入れておくという方法もあります。もし、Cartクラスの外からカートIDを参照したい場合は、getCartId()のようなゲッターメソッドで参照します。

global文の利用は、ほかの代替手段で対応できる場合がほとんどです。今となってはglobal文を利用しないといけない場面はほぼないといってよいでしょう。

6. 配列の操作

配列の操作においてもなかなか特徴があります。

まず、23行目では$a_Itemというメンバ変数を初期化しているのですが、下記のように空文字を代入して初期化しています。この行だけを見ると$a_Item変数は、ハンガリアン記法による配列を想定しているのか、代入値である文字列を想定しているのかがわかりません。

    var $a_Item = "";

そのあとのコードを見ると配列として操作しているので、ここではハンガリアン記法の勝利となるのですが、配列の初期化を行うなら、以下のように素直に[]を渡したほうがよいです(ここではアクセス修飾子にprotectedを設定しています⁠⁠。

    protected $a_Item = [];
    // PHP 5.3未満
    protected $a_Item = array();

次に114行目では、while文にて$this->a_Itemの値を順に参照しています。PHP 3当時はforeach文が存在しなかったので、こういった配列の添字をインクリメントして各要素の値を参照する手法が多く取られていました。通常、配列を順に格納していけば添字は連続した値になるため、正常に動作するように思えます。ただ、もし添字に欠番があったり、添字は存在していても値がnullfalseなどの場合、while文がその時点で終了してしまいます。

配列を順に参照していくのであれば、次のようにforeach文を使うのがよいでしょう[2]⁠。

    foreach ($this->a_Item as $k => $v) {
        (snip)
    }

なお、元のコードを実行すると、while文の個所で最低でも1回はNoticeエラーが出ます。なぜNoticeエラーが出るのかは考えてみてください[3]⁠。

治療すべきポイントを洗い出したところで、次回はこれらの問題点を治療していきます!

おすすめ記事

記事・ニュース一覧