LoginSignup
324
343

More than 5 years have passed since last update.

PHPで出来てしまうが控えたほうががいい書き方

Last updated at Posted at 2017-01-10

正常系は上手く動くし問題ない!と思ってもケースによっては危なかったり
あんまりよくない書き方ってあると思います。
そういうような控えたほうががいいような書き方をケース別にまとめたものです。
まばらには書かれているんですが、一つの場所にはまとまってないのでメモがてら。

==での比較

違う言語から戻ってくると間違って書いてしまいがちな判定。

問題点

== の場合は 型の相互変換をかけてから比較するので
意図しない比較が行われる可能性がある
PHP: 型の相互変換 - Manual

コード例と解説

以下のようなコードがあるとします。

if ($target == 1) {
    echo 'this is 1';
}

もちろん target = 1; のときにも通るのですが
== だと1以外のものでも判定が true になってしまうものが結構あります。
true になってしまう例としては以下。

$target = "1";
$target = true;
$target = "1aaa";

// 上記はすべてtrueになる

すべて型の相互変換(暗黙の型変換1)が原因
型の相互変換を頼りたい場面(そんなにないと思いますが)以外は
=== を使った方がいいと思います。

空文字判定としてのempty

問題点

empty は文字通り空を判定するが
空と定義する範囲が広いので、思わぬものも と認識されてしまう。

コード例と解説


// $targetが空文字の場合はデフォルト値を入れる
if (empty($target)) {
    $target = "default";
}

一見大丈夫そうに見えるのですが
empty() は 空文字以外にも色々 true 扱いしてくれるので
範囲が広くて危険です。

PHP: empty - Manual

次のような値は空であるとみなされます。
"" (空文字列)
0 (整数 の 0)
0.0 (浮動小数点数の 0)
"0" (文字列 の 0)
NULL
FALSE
array() (空の配列)
$var; (変数が宣言されているが、値が設定されていない)

0という文字も空扱いになってしまいます。
そのため、「何も文字が入って来ないときは変わりに文字を代入」のような処理の場合は
空白比較をしたほうが良いです。

改訂例
if ($target === "") {
    $target = "default";
}

bool型変数以外の判定への利用

問題点

変数の中身があるかどうかを判定するのに if ($var) みたいな書き方をしますが
変数の型によっては思わぬものを返すので控えたほうがよい。

コード例と解説

$typeNo = $params['type'];
$message = "type is no setting";

// $params["type"]が指定されていれば表示
if ($typeNo) {
    $message = "typeNo is " . $typeNo;
}
return $message;

$typeNo に何かしら入っていればそれを表示したいプログラムです。
変数をそのまま条件式に利用した場合、論理型への型変換がかかります。

jsonやクエリーストリングから起こした配列を処理する時に
『指定されたキーが無ければnullが返ってくるのでfalseになる』
という挙動を期待して上のような書き方をするケースが見受けられます。

ただ、この場合の論理型への変換が癖があるので思わぬ事態を招きやすいです。

リファレンスだと型変換の挙動は以下の通り。
PHP: 論理型 (boolean) - Manual

boolean に変換する場合、次の値は FALSE とみなされます。

boolean の FALSE
integer の 0 (ゼロ)
float の 0.0 (ゼロ)
空の文字列、 および文字列の "0"
要素の数がゼロである 配列
特別な値 NULL (値がセットされていない変数を含む)
空のタグから作成された SimpleXML オブジェクト

そのため、今回は $params["type"] には数字が入ってくる可能性がある場合
0 が入ってくると false 扱いになり
意図しない動作となってしまいます。
結構論理値への変換は勘違いしやすいので注意が必要です。

以下、それを踏まえた改善案の例です。

改善案
$message = "type is no setting";

// $params["type"]が指定されていれば表示
if (isset($params["type"])) {
    $typeNo = (int)$params['type'];
    $message = "typeNo is " . $typeNo;
}
return $message;

なお、三項演算子でも同じことをやりがちなので注意

// ステータスの状態を数字で管理
// デフォルト値
const DEFAULT_STATUS_NO = -1;
// 待機状態
const STAY_STATUS_NO = 0;
// 動作中
const DOING_STATUS_NO = 1;

// 中略

$jsonArray = json_decode($json, true);

// ステータスNoがセットされていれば値を引き継ぎ
// そうでなければデフォルト値をセット
$statusNo = $jsonArray['status'] ? $jsonArray['status'] : DEFAULT_STATUS_NO; 

前述のように0false扱いになってしまいおかしくなるケース。
三項演算子の省略形 ?: 等でデフォルト値を埋める処理をしている場合は
注意が必要です。

配列の宣言なしでの代入

問題点

変数に対して記述のしかたと代入する場所の状態によって
よしなに配列を自動で生成してくれるが
既に何か入っていると代入が失敗したりするので
思わぬバグを生む可能性がある。

コード例と解説

下記のような処理があるとする。

$array = [];
$array['flower']  = "yeah";
$array['star'][1] = "hoge";
$array['star'][2] = "hoge";

$array['star'] = [] とわざわざ宣言しなくても
配列ができあがってくれます。
簡単な処理なら便利なのですが
以下のようになってしまうと値が入らなくなってしまいます。

$array = [];
$array['flower']  = "yeah";
// string型のものを入れる
$array['star']    = "string!";
// すでにstringが入っているので代入できない
$array['star'][1] = "hoge";
$array['star'][2] = "hoge";

そのため、できるだけ配列をネストしたりする場合は
空配列を宣言しておいたほうが明示的でわかりやすくなるので安心です。

改訂例
$array = [];
$array['flower']  = "yeah";
// string型のものを入れる
$array['star']    = "string!";

// 新たに配列を代入したい場合は以下
$insertArray = [];
$insertArray[1] = "hoge";
$insertArray[2] = "hoge";
$array['star']  = $insertArray;

arrayを引数とするfunction

問題点

arrayの変数を引数にしてfunctionを作ると
本来必要のない情報が渡ったり、配列の状態を細かく判定する必要が出てくるので
本来やりたい処理とは異なる部分に気を使う必要が出て来る

コード例と解説

以下のような処理があるとする。

public function jsonToProfileText($json)
{
    // jsonをデコードしてarrayに
    $student = json_decode($json, true);
    return $this->createProfileText($student);
}

private function createProfileText($student)
{
    $name = $student['name'];
    $age  = $student['age'];
    return "My Name is" . $name . ", " . $age . " years old.";
}

arrayにすべてのデータが入ってて、そのまま渡したほうが楽なので引きまわしてます。

この場合、配列なので取り出したいキー(上記の場合だと nameage)が
なかったりする場合があります。

その場合配列の中にキーが存在するかまで気にしないといけないので
もし厳重にチェックしたい場合は配列のキーチェックなどが必要になります。

でも createProfileText は文章を作る処理なのにキーを気にするのは
文章作るという作業の範疇を超えている気もします。

渡してあげる文字があるかを気にするのはどちらかと言うと呼び出し側なので
呼び出し側で見てあげて、文章を作る方には必要な要素だけ渡してあげると
createProfileText は文章をつくることに集中できます。

上記のことを踏まえて直すと以下のようになります。

public function jsonToProfileText($json)
{
    $student = json_decode($json, true);
    // 必要であればここで $student['name'] と $student['age'] の状態を確認する
    return $this->createProfileText(
        $student['name'],
        $student['age']
    );
}

private function createProfileText($name, $age)
{
    return "My Name is" . $name . ", " . $age . " years old.";
}

こちらに関してはケースバイケースなので
必ず分解してパラメータを渡そう!ってことではないですが
考え方として頭に入れておくと空チェックなどが集約できますし
見通しがよくなることがあります。

URLを指定した file_get_contents() でのコンテンツ取得

問題点

インターネット上のコンテンツなどもURL指定をすれば取得できるのですが
file_get_contents() でコンテンツが取得できなかったときのハンドリングをしようとすると
考慮しなければいけない癖が多いのでオススメできないです。

コード例と解説

長文になってしまったので別記事にまとめました。
APIなどにfile_get_contents()を使うのはオススメしない理由と代替案 - Qiita

参考URL

まとめる上で参考したページ

その他有用Tips


  1. 型の相互変換が正しい呼び方。ありがちなケースに関しては参考ページにも記載しているこちらがわかりやすいです。 

324
343
3

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
324
343