「保守性・管理性が劇的に上がるPHPのスマートなコードの書き方12選」の解説その1

以下のような記事が昨日話題になりました。

はてなブックマーク - 保守性・管理性が劇的に上がるPHPのスマートなコードの書き方12選 | BULK SERVER blog

現在では記事自体は削除されていますが、魚拓がとられているのでまだご覧になってない方は以下のリンクをどうぞ。

http://bulkserver.jp/blog/2014/08/07/php-code/ - 2014年8月12日 09:28 - ウェブ魚拓

すでに消されてる記事に対して、アレコレ言うのはちょっと悪趣味かなとも思ったのですが、ブコメを見ると「もうすこし優しく教えてあげなよ」「括弧の省略がなんで嫌いなんだろう」といった記述があったので書いてみることにしました。

元記事を書いた方にこのエントリが読まれることを切に願います。

自分の立場

なんというか某記事、そんなにDisりたくないし、自分だって間違った理解でブログ書いちゃうことあるんだけど、入門書一冊でも読めばわかりそうなものを堂々と記事してるというのと、それが個人ブログじゃなくて会社のブログであるという点がとてもアレで
— 末並晃＠半日断食実践中 (@a_suenami) 2014, 8月 12

そのせいで全うなPHPerまで「これだからPHPは…」って言われるようになるの容易に想像できるので今回ばかりはちゃんと批判しないとダメな気がした。
— 末並晃＠半日断食実践中 (@a_suenami) 2014, 8月 12

こういった立場でこのエントリ書いてます。

元記事を Dis りたいわけではないですが、まともな PHPer の風評被害に繋がるのもよくないと思ったので、僕が間違ってると思った点をサンプルコードを交えながら解説していきたいと思います。

1．括弧の省略

括弧の省略がよくないというのは僕の中では当たり前だと思っていたのですが、

括弧の省略を積極的にするべきというコーディング規約もある
ブコメを見ると「何故ダメなのかわからない」という人がいる

というようなので簡単に解説します。

解説と言っても難しいことを言うつもりはなくて、一言で言うと if 節にあとから修正が加えられたときに不具合を混入する可能性があるというのが括弧の省略がよくないとされる理由です。

例えば以下のようなコードを考えます。

<?php

function output($flg) {
  if ($flg)
    print "foo\n";
}

print "Output flag is true\n";
output(true);

print "Output flag is false\n";
output(false);

引数として渡される出力フラグが true であれば標準出力に文字列を出力し、そうでない場合は何もしないというシンプルな関数です。

実行結果は次のようになります。

$ php output.php
Output flag is true
foo
Output flag is false

期待通りの結果ですね。

では、このコードを以下のように変更しましょう。出力フラグが true の場合に表示する文字列を追加しました。

<?php

function output($flg) {
  if ($flg)
    print "foo\n";
    print "bar\n"; // この行を追加
}

print "Output flag is true\n";
output(true);

print "Output flag is false\n";
output(false);

この実行結果はこうなります。

$ php output.php
Output flag is true
foo
bar
Output flag is false
bar

文字列 bar は出力フラグに関係なく表示されてしまっています。これは元記事でも説明されている通り、

命令が二つ以上であれば括弧でくくる必要があります。

のためです。

かの有名な Apple でさえ、半年ほど前、これに起因するセキュリティバグを出してしまいました。

Apple史上最悪のセキュリティバグか、iOSとOS XのSSL接続に危険すぎる脆弱性が発覚──原因はタイプミス？ | アプリオ

Python のようにインデントでブロックを表現する言語でない以上、ブロックを表現する手段は中括弧のみであり、それを省略することは Apple の例のみでなく多くの不具合混入の可能性を残すことになりますし、それが保守性をあげることになるとはとても考えにくいです。なので、括弧は省略するべきではないのです。

ただ、例外的に関数やメソッドのガード節は括弧を省略してもいいかなと個人的には思っています。

<?php

function someFunction($argument) {
  if ($argument < 0) return false;

  // ...実際の処理...
}

ただし、これも省略してもよいと考えている程度であり積極的に省略すべしとは思わないですし、あくまで個人的意見なのでチームの採用しているコーディング規約に従うべきだと思います。

以下のようにしても決して可読性は下がらないと思いますし、こちらのほうが安心感はあります。

<?php

function someFunction($argument) {
  if ($argument < 0) { return false; }

  // ...実際の処理...
}

2．三項演算子

三項演算子自体は確かにスマートなシンタックスだと思いますし、僕自身もそれなりの頻度で利用しています。そういう意味で元記事にそこまでの反論はありません。

ただ、ひとつ注意しておいて欲しいと思うのは、三項演算子の多用はかえって可読性を下げることもあるということです。

以下のコードを比較してみてください。

<?php

// 関数名が非常に長い関数の戻り値を代入
$var = $someObject->methodHavingVeryVeryLongName() ? $someObject->methodHavingVeryVeryLongName() : 'This is a default value' ;

// if で書いた場合
if ($someObject->MethodHavingVeryVeryLongName()) {
  $var = $someObject->MethodHavingVeryVeryLongName();
} else {
  $var = 'This is a default value';
}

// 三項演算子のネスト
$var = $condition1 ? ($condition2 ? 'Both condition1 and condtion2 are true' : 'Condtion1 is true but condition2 is false') : 'condition1 is false';

// if で書いた場合
if ($condtion1) {
  if ($condtion2) {
    $var = 'Both condition1 and condtion2 are true';
  } else {
    $var = 'Condtion1 is true but condition2 is false';
  }
} else {
  $var = 'condition1 is false';
}

世の中にはワンライナー厨と呼ばれる人たち（複雑な処理を1行で書くことに快感を感じる人たち）がいて、そういった人は容易に上記のようなコードを書きます。僕の経験上、三項演算子は必要としないところでもそれを利用してバランスを欠いた状態*1に陥りやすいと感じており、if よりも三項演算子のほうが保守性を向上させるかどうかはケースバイケースだと思っています。

元記事が「何が何でも三項演算子のほうがスマートなんだ！！」と言っているわけではないというのは重々承知していますが、だからこそ「こういう書き方もあるよ」くらいにとどめ、「保守性が向上する」とまで大風呂敷を広げないほうが懸命かなと感じました。

3．switch文

switch 文がどうしても必要なケースで利用するのはよいと思います。

ですが、switch 文はマーチンファウラーの著書「リファクタリング」において「コードの不吉な臭い」として挙げられており、それを必要とした時点で設計の失敗を疑うべきです。

リファクタリング―プログラムの体質改善テクニック (Object Technology Series)

作者: マーチンファウラー,Martin Fowler,児玉公信,平澤章,友野晶夫,梅沢真史
出版社/メーカー: ピアソンエデュケーション
発売日: 2000/05
メディア: 単行本
購入: 94人クリック: 3,091回
この商品を含むブログ (311件) を見る

新装版もあります。（こちらは僕自身もまだ読んでいません…）

新装版リファクタリング―既存のコードを安全に改善する― (OBJECT TECHNOLOGY SERIES)

作者: Martin Fowler,児玉公信,友野晶夫,平澤章,梅澤真史
出版社/メーカー: オーム社
発売日: 2014/07/26
メディア: 単行本（ソフトカバー）
この商品を含むブログ (4件) を見る

マーチンファウラー氏の主張は、switch（if もですが）は多くの場合、ポリモーフィズムに置き換えられるのではないかということです。現在ではPHPもオブジェクト指向言語としての機能を持っているのでこの考え方に則ってコードを設計することは可能です。

例えば以下のコードを見てください。

<?php

class Person
{
  private $_name;
  private $_sex;

  public function __construct($name, $sex) {
    $this->_name = $name;
    $this->_sex  = $sex;
  }

  public function getName() {
    return $this->_name;
  }

  public function getSex() {
    return $this->_sex;
  }
}

$male = new Person('Bob', 'male');
$female = new Person('Alice', 'female');

foreach ([$male, $female] as $person) {
  switch ($person->getSex()) {
    case 'male':
      print "Hello, Mr. {$person->getName()}.\n";
      break;
    case 'female':
      print "Hello, Ms. {$person->getName()}.\n";
      break;
  }
}

対象の人が男性か女性かに応じて、挨拶文を変えるという分岐処理ですね。これは以下のように書き直せます。

<?php

abstract class Person
{
  private $_name;

  public function __construct($name) {
    $this->_name = $name;
  }

  public function getName() {
    return $this->_name;
  }

  abstract public function getSex();
  abstract public function getGreetingMessage();
}

class Male extends Person
{
  public function getSex() {
    return 'male';
  }

  public function getGreetingMessage() {
    return "Hello, Mr. {$this->getName()}.\n";
  }
}

class Female extends Person
{
  public function getSex() {
    return 'female';
  }

  public function getGreetingMessage() {
    return "Hello, Ms. {$this->getName()}.\n";
  }
}

$male = new Male('Bob');
$female = new Female('Alice');

foreach ([$male, $female] as $person) {
  print $person->getGreetingMessage();
}

Personクラスを抽象クラスとし、性別ごとにサブクラスを新たに作成しました。

これによって挨拶文を出力する部分から条件分岐を消すことができました。プログラミングにおいてバグが混入されてしまうポイントとして条件分岐は非常に多いかと思いますので、if や switch をできるだけ排除し、ポリモーフィズムに置き換えることで保守性は向上すると言えるかと思います。今回の例は性別による分岐だったので、あとから条件が増えることは考えにくいですが、仮に仕様変更によって条件が増えた場合、ポリモーフィズムによる実装であればサブクラスを追加するだけで済み、既存の機能への影響を最小化することができます。

PHPのインスタンス生成コストに注意

コンパイル言語である Java と違い、PHP は多くの場合に性能の壁が立ちはだかりますし、とりわけインスタンス生成に関するコストは無視できないと言われています。（要出典）

したがって、if や switch はコード中に一切ないというのが理想的ではありますが、それを厳密に実践しようとすると性能の壁に悩む事になるかと思いますので、注意してください。何事もバランスです。

4．for文

while を知っていて for を知らない人はあまりいないと思っているのですが、一応こちらもとりあげておきます。

while や for でのループ処理を否定するつもりはありませんが、PHP を使っていると圧倒的に foreach を使うケースのほうが多いのではないでしょうか？PHP においてループ処理をやりたくなるのがほとんどの場合、配列や連想配列に対して何らかの処理をすることに起因します。

foreach を利用すると

<?php

$arr = [1, 2, 3, 4, 5];

for ($i = 0; $i < count($arr); $i++) {
  $arr[$i] = $arr[$i] * 2;
}

var_dump($arr);

は

<?php

$arr = [1, 2, 3, 4, 5];

foreach ($arr as $key => $val) {
  $arr[$key] = $val * 2;
}

var_dump($arr);

のように書き直すことができます。

実行結果はいずれも

$ php loop.php
array(5) {
  [0]=>
  int(2)
  [1]=>
  int(4)
  [2]=>
  int(6)
  [3]=>
  int(8)
  [4]=>
  int(10)
}

となります。

ただし、実際のところもっとよい方法もあります。

ループ処理は多くの場合

map
- 配列や連想配列から別の配列や連想配列を作る
filter
- 配列や連想配列の要素をある条件で絞り込む
fold
- 配列や連想配列を畳み込んで、ひとつの値とする

の3つの大別できます。*2

PHP でも一応これらの機能を提供する関数はあって、それを用いれば foreach すら必要なくなります。

<?php

// 対象の配列
$arr = [1, 2, 3, 4, 5];
$doubleArr = [];
$evenArr = [];
$sum = 0;

/**
 * map
 */
function toDouble($x) {
  return $x * 2;
}

// foreach を用いた場合
foreach ($arr as $key => $val) {
  $doubleArr[$key] = toDouble($val);
}

// arrya_map を用いた場合
$doubleArr = array_map('toDouble', $arr);

/**
 * filter
 */
function even($x) {
  return $x % 2 == 0;
}

// foreach を用いた場合
foreach ($arr as $key => $val) {
  if (even($val)) {
    $evenArr[] = $val;
  }
}

// array_filter を用いた場合
$evenArr = array_filter($arr, 'even');

/**
 * fold
 */
function sum($current, $value) {
  return $current + $value;
}

// foreach を用いた場合
foreach ($arr as $key => $val) {
  $sum = sum($sum, $val);
}

// array_filter を用いた場合
$sum = array_reduce($arr, 'sum', 0);

これらの関数を用いることによって、map に適用する処理や filter の条件式、fold の畳み込み式を関数として切り出し名前をつけることができますし、コードから手続き的な記述を減らすことができ、保守性の向上に貢献することができます。

続く

ここまで書いてずいぶんと長くなってしまったので、残りについては別エントリにしたいと思います。

*1:「プログラマの麻疹」と呼ばれますね。

*2:関数型言語の経験がある方はご存知だと思いますが、もし興味がある方は「List モナド」などでぐぐってみてください。