「保守性が劇的に上がるPHPのスマートな書き方12選」ネタにマジレスしてみる件

はてブで「保守性が劇的に上がるPHPのスマートなコードの書き方12選」という記事がホットエントリーに入っていたので見たのですが、素人の僕から見てもヤバい内容でした。はてブコメント欄を見ても同じような反応でした。もう元記事は削除されてしまっているのですが、それぞれの書き方について、自分の考え方を書いておきたいと思います。一応保険をかけておきますが、僕は趣味でPHPを触っているくらいで、本業ではやってません。

かっこの省略

一般的に、if文は次のように書きます。

if ( 条件 ) {
    条件を満たしていたときの処理
}

しかし、条件を満たしていたときの処理が1つしかない場合は、処理を囲っているかっこを省略することができます。

if ( 条件 ) 
    条件を満たしていたときの処理(1行)

しかし、これは「省略することができる」というだけで、実際にはやらないほうがいいです。なぜなら、次のような場合、判断に困るからです。

if ( $a == 2 ) 
    $a = 0;
    echo $a;

この場合、「echo $a;」は、$aが2の時だけやってほしいのか、それともどんなときでもやってほしいのかがわかりません。つまり、かっこを「わざと」省略しているのか、「間違って」つけわすれてしまったのかが判断できません。どちらか判断するには、どういう処理をしているかをたどっていく必要があり、保守性が高いとは言えません。なので、かっこの省略はよくないです。

もし、省略するとしたら、こういう使い方ならまだ許されるかもしれません。

if ( $a == 2 ) $a = 0;
echo $a;

これなら、「わざと省略した」という感じがして、いいと思います。が、基本は使わない方が安全だと思います。

三項演算子

「こうならこれ、ちがったらあれ」をif文でやればこうなります。

if ( $a % 2 == 0 ) {
    echo "偶数";
} else{
    echo "奇数";
}

三項演算子を使えば、もっと短く書けます。

echo ( $a % 2 == 0 )? "偶数": "奇数";

これは、1行が短い場合は使ってもいいと思いますが、長くなると読みにくくなるので、その場合は、if文を使った方がいいと思います。特に、三項演算子をネストして使うと、なにがなんやらわからなくなります。書いた直後は読めるけど、時間がたてばよくわからなくなります。ぱっとみてどんな処理が行われているか、すぐにわかるようであれば使ったらいいと思います。

switch文

「else if」がたくさんあるif文は、switch文で書くことができます。個人的には「else if」をいっぱい書けばいいじゃないかって思うんですけど。まぁ、一応switchも使えます。

ただ、PHPで気をつけないといけないのは、switch文では「緩やかな比較」が行われるということです。たとえば、次のプログラムを見てみます。

switch ( $a ) {
    case 0:
        echo "0です";
        break;
    case 1:
        echo "1です";
        break;
    default:
        echo "0でも1でもない";
}

$aが0なら「0です」が表示され、$aが1なら「1です」が表示され、それ以外は「0でも1でもない」が表示されるはずです。しかし、例えば「$a=""」とか「$a=false」などの場合も「0です」が表示されてしまいます。これは、「緩やかな比較」が行われているからです。

もし、こういった罠にはまる可能性がある場合は、(おススメしませんが)次のように書くこともできます。

switch ( true ) {
    case $a === 0:
            echo "0です";
            break;
        case $a === 1:
        echo "1です";
        break;
    default:
        echo "0でも1でもない";
}

switchの()内はtrueにして、caseで「===」によって型も考慮した厳密な比較を行っています。ただ、このような意味深なものは書かない方がいい気がします。

始めからif文と===を使えばいいんじゃないのかな、と個人的には思ってしまいます。

for文

繰り返しは、while文を使っても、for文を使っても書けます。個人的には、for文の方がよく書くような気がします。ループ回数が決まっているものはfor、決まってないものはwhileを使うことが多いですね。

ただ、配列のすべての要素に対して何かをする、という場合は、配列の個数がわからなくてもcount関数を使えばいいので、for文を使うことが多いと思います。次のようなコードにいろんなところで出くわします。

for ( $i = 0; $i < count($member); $i++ ) {
    // 繰り返し処理
}

ただ、細かいですが、「count(配列名)」は別の変数にいったん入れておいた方がいいですね。何度もcountを呼ぶのは無駄だし、時間がかかりますからね。

別にfor文を使ったからって、保守性がどうとか言うことはない気がします。

配列への要素追加

次の2つの処理は、配列の最後に要素を追加するためのもので、同じ処理をしています。

array_push( $array, $value );
$array[] = $value;

下の方が簡単だし、いちいち関数を呼ぶこともないため処理が早くなるので、下のやり方がいいと思います。これは元記事通りかなと思います。

引数のデフォルト値

関数を作るときに、デフォルト値を設定することができます。

function myFunction( $a, $b = デフォルト値, $c = デフォルト値 ) {
    // 処理
}

ここで、注意が必要なのは、引数は、デフォルト値を設定していないものは左に、デフォルト値を設定しているものは右におかないといけないということです。なので、元記事にあったような、次の書式は使えません。

function myFunction( $a = デフォルト値, $b = デフォルト値, $c ) {
    // これはダメ
}

もし、2つ目の書式でかいた場合、$cに値を入れ、$aと$bはデフォルト値を使おうと思って「myFunction($cへの値)」とすると、$cへの値が$aに入り、$cへは何も値が入らずにエラーとなります。

デフォルト値の設定自体が保守性を上げるかは微妙ですが、設定できるのであれば設定しておいた方が楽かなと思います。

関数でのglobal宣言

関数内で、関数外の変数に対してglobal宣言を行えば、その関数の中でその変数を使うことができるようになります。これは便利ですが、とても危険です。

global宣言をいたるところでしていたとすると、仮に意図していないところで変数の値が変わっていた場合に、どこで値が変わっているのか犯人を捜すのがとても大変です。保守性の点から、安易なglobal宣言はやってはいけません。

条件式での変数定義

if文の条件式で、変数定義をしながら比較もする、ということができます。元の記事で上がっていたのは、こういう例でした。

if ( $num = strpos( $str, 'abc' ) ) {
    echo '「abc」はこの変数の' . $num . '番目にあります。';
}

これはしない方がいいと思います。後で見たときにif文の条件式を見て「=じゃなくて==の間違いかな?」と誤解されるかもしれません。一般的には、if文の条件式では、比較を行うからです。なので、めんどうでも、代入と比較は分けた方がいいです。

文字列への変数挿入

文字列と変数を組み合わせた文章について。普通は「.」(ピリオド)で連結します。

echo '私は'.$name.'と言います。年齢は'.$old.'歳です。よろしくおねがいします。';

元記事では、変数の展開を行って、次のように書くことを勧めていました。

echo "私は{$name}と言います。年齢は{$old}歳です。よろしくおねがいします。";

個人的には、ピリオドでつなげる方がいいと思います。というのは、ピリオドの前後にスペースをいれれば、ある程度見やすくなると考えているからです。2つ目の方法では、スペースを開けるとスペースがそのまま反映されてしまいます。日本語と英語の間がつながっているとみにくいと思うので、個人的にはおススメしません。

が、これは個人の好みのレベルかな、と思います。

empty

issetやemptyの話が出るのは、たいていユーザーの入力値の処理の話をしているときです。

たとえばユーザーがフォームで入力して、そのデータを受け取って$_POST[‘email’]なんかを取得しようとした場合、何もpostされずに変数が未定義だった時はエラーが出てしまいます。なので、処理をする前に、定義されているかどうかの確認が必要なわけですね。

issetは、変数が存在していて、null以外の値であればtrue、それ以外はfalseとなります。空文字もtrueになるため、これを避ける場合は次のようにチェックします。

if ( isset( $_POST['email']) && $_POST['email'] != '' ) {
    // 処理
}

似たようなemptyを使う方法もありますが、実は少し挙動が違います。

if ( ! empty( $_POST['email'] ) ) {
    // 処理
}

emptyは、変数が存在しない場合や変数の値がfalseに等しい場合に、trueを返します。なので、上のif文の処理が行われるのは、「変数が存在して、変数の値がfalseと一致しないとき」です。空文字のときはfalseと一致するとみなされるため、処理されません。それはいいのですが、例えば0が入力された時も処理されません。ユーザーが0と入力しても「入力されていません」という反応になってしまいます。なので、0の時の処理も必要になります。

そもそもif文の条件式に「!」が出てくると、条件を考えるときに間違いやすいので、個人的にはissetを使ったやり方をおススメしたいです。

再帰関数

関数の中から自分を呼んでくるという関数を再帰関数といいます。

function test($num){
    if($num > 0){
        return $num + test($num - 1);
    }else{
        return 0;
    }
}

上は、1から$numまでの和を返す関数です。再帰関数は、計算速度も遅いし、メモリをたくさん使うし、そもそも書くのも読むのも難しいので書かない方がいいのではないかと思います。再帰関数を使う方が処理が簡単にかけるという場合のみ、使えばいいと思います(どういう状況か、ぱっと思いつかないけど)。

結合代入演算子

「$a = $a + 2」を「$a += 2」と書けるように、「$a = $a . $b」は「$a .= $b」と書けます。これは、使えばいいかなぁと思います。保守性にはあまり影響を与えない気がしますが。

まとめ

というわけで、まとめると、

かっこの省略⇒省略しちゃダメ、三項演算子⇒使ってもいいけど見にくくなるならダメ、switch文⇒緩やかな比較になることに注意すれば使ってもいい、for文⇒使う(保守性関係ない)、配列への要素追加⇒使う、デフォルト値⇒使う、global宣言⇒安易に使わない、条件式での変数定義⇒ダメ、文字列への変数挿入⇒どっちでもいい、empty⇒ダメじゃないけどissetのほうがいい、再帰関数⇒無理して使わない、結合代入演算子⇒使う(保守性関係ない)

という感じになるかと思います。

そもそもですが、僕は元記事に関しては、小手先のテクニックではなく、リファクタリングのことを考えるとか、どうやって設計するかとかそういうのを期待していたので、そういう記事を誰か書いて下さい。。。。

前の記事:
新しい変数を使わずに、2つの変数を入れ替える方法
次の記事:
PHPUnit・スケルトンジェネレータのインストールでつまづいたメモ