【PHP】empty()をカジュアルに使うな問題

引き継いだプログラムがあらゆる場所で empty() で存在の評価をやってて吐くかと思いました

カジュアルにempty()を使っている例

if (!empty($user->name)) {
   //なんらかの処理
}

if (!empty($request->name)) {
   //なんらかの処理
}

こういうコードをホイホイ書いちゃうからPHPerは信用されないんだよ……

何が問題なのか

empty() というのは、値がNGな雰囲気を持ってるかどうかをなんとなく判断する関数で、入力値が限定されている場面やあんまり厳密な判断を必要としない場面ではとても便利なのですけど、PHP特有の緩い判断を行うので安易に使うとバグの元になります。


empty() に何を入れるとどう判断されるかのまとめがこちらです。



（https://qiita.com/shinichi-takii/items/00aed26f96cf6bb3fe62 から引用）



これを見てわかってもらえると嬉しいのですが先ほどのようなコードを書く人はこれを見てもわかってくれないのですよね……

empty()は値が無いことをチェックする関数ではない

1つ目のコードはユーザー名があったらなんか処理をするということを期待したコードですが、もしユーザー名が「0」だったら empty(0) === true になるので処理が行われません。

if (!empty(0)) {  //empty()はtrueを返す
   //なんらかの処理
}

同様に、なんらかのフォームの入力値をチェックし存在していたらなんか処理をしたいというのが2つめのコードですが、入力した名前が「0」さんだったら処理が行われません。

どう書けば正しいか

モデルに関しては状況によって変わると思いますが、例えばこう。

if (is_null($user->name) === false) {
   //なんらかの処理
}

if ($request->has('name')) {
   //なんらかの処理
}

PHP使ってると入力値の型が何とか、どんな値を取りうるとか忘れがちなのはあるあるなのですけど、でもPHPにそういう概念がないかというとそういうわけではなくて、きちんとやる方法は提供されています。使う人間の方が拙いだけです。

もちろん解って（きちんと対策を取った上で）使っているなら便利な関数なので、使うなということではないのですけど、逆に言えば使うんだったらちゃんと解って使ってくださいねってことであり、カジュアルに「empty() で何でもいけんで！」とか言うと普通のエンジニアなら嫌な顔するよねということです。



いやー、、最近のPHPerはこんな書き方しないと思うんですけどねえ。これ以外にもイカしたクソ記法満載のコードなので、どこかで身につけてしまった手癖なんだろうなあ。