質問をすることでしか得られない、回答やアドバイスがある。

15分調べてもわからないことは、質問しよう!

新規登録して質問してみよう
ただいま回答率
85.50%
PHP

PHPは、Webサイト構築に特化して開発されたプログラミング言語です。大きな特徴のひとつは、HTMLに直接プログラムを埋め込むことができるという点です。PHPを用いることで、HTMLを動的コンテンツとして出力できます。HTMLがそのままブラウザに表示されるのに対し、PHPプログラムはサーバ側で実行された結果がブラウザに表示されるため、PHPスクリプトは「サーバサイドスクリプト」と呼ばれています。

Q&A

解決済

4回答

6306閲覧

PHP 配列の変数の初期化のタイミングについて

pgm_bakabon

総合スコア61

PHP

PHPは、Webサイト構築に特化して開発されたプログラミング言語です。大きな特徴のひとつは、HTMLに直接プログラムを埋め込むことができるという点です。PHPを用いることで、HTMLを動的コンテンツとして出力できます。HTMLがそのままブラウザに表示されるのに対し、PHPプログラムはサーバ側で実行された結果がブラウザに表示されるため、PHPスクリプトは「サーバサイドスクリプト」と呼ばれています。

0グッド

0クリップ

投稿2019/01/17 06:32

編集2019/01/17 06:39

はじめに

PHPのコードレビューをしていて、配列の変数の初期化のタイミングを指摘したいのですが、
言語化ができていないので、指摘できない状態にいます。
言語化の手助けをお願いします。

元のコード(サンプルのため変数名などは簡略化してます)

PHP

1$ids = getIds(); 2$datas = getDatas(); 3 4// $datasが空でforeachに入らないケースがあります 5foreach ($datas as $key => $val) { 6 $dataIds[] = $val['id']; 7} 8 9if(empty($dataIds))$dataIds = array(); 10$mergeIds = array_merge($ids, $dataIds);

指摘して以下のように直すべきと考えているコード

PHP

1$ids = getIds(); 2$datas = getDatas(); 3 4// ※指摘点 foreachの前に配列の初期化をしておきたい 5$dataIds = array(); 6// $datasが空でforeachに入らないケースがあります 7foreach ($datas as $key => $val) { 8 $dataIds[] = $val['id']; 9} 10 11$mergeIds = array_merge($ids, $dataIds);

求めていること

「foreachの前に配列の初期化をしておきたい」と伝えたいのですが、
なぜforeachの前なのかの説明が言語化できていないので、ここの言語化をお願いしたいです。
メソッドの冒頭や、変数を使う直前で初期化を当たり前にしてきてしまったという背景があります。
よろしくお願いいたします。

気になる質問をクリップする

クリップした質問は、後からいつでもMYページで確認できます。

またクリップした質問に回答があった際、通知やメールを受け取ることができます。

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

guest

回答4

0

ベストアンサー

「利用する前に初期化をしておく癖をつけましょう」でいいのではないでしょうか。
既に「$datasが空でforeachに入らないケースがあります」と述べられていますし、それ以上の理由はないかと思います。
配列を初期化せずにいきなり利用した場合のエラーを伝えても良いですし。

細かいですが、配列初期化・定義は下記のほうが今は良いです。

php

1$dataIds = [];

投稿2019/01/17 06:40

m.ts10806

総合スコア80765

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

pgm_bakabon

2019/01/17 07:14

ご回答ありがとうございます。 「利用する前に初期化」という単純明快な言葉を使わさせていただきレビュー指摘をしました。 また合わせて、 if(empty($dataIds))$dataIds = array(); というロジックが入ることで可読性が悪くなるという面も伝えることにしました。
m.ts10806

2019/01/17 07:20

解決されたようで何よりです。 根拠資料であればPHPマニュアル以上に最適なものはないので別の回答とあわせてレビュー結果すると良いです。
guest

0

こういうのはarray_mapなどで取るのがラクでは?

PHP

1$datas=[ 2 ["id"=>1,"data"=>"data1","other"=>"other1",], 3 ["id"=>2,"data"=>"data2","other"=>"other2",], 4 ["id"=>3,"data"=>"data3","other"=>"other3",], 5 ]; 6$dataIds=array_map(function($x){ 7 return $x["id"]; 8},(array) $datas); 9 10print_r($dataIds);

かりに$datasが空配列でも、配列以外のなにかでも$dataIdsには
空配列が返ります

投稿2019/01/17 07:52

yambejp

総合スコア114572

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

0

なぜforeachの前なのかの説明が言語化できていないので、ここの言語化をお願いしたいです。

PHPマニュアルにもその旨が明記してあります。

$arr がまだ存在しない場合は、新しく作成します。 つまり、これは配列を作成する方法のひとつでもあります。 とはいえ、この方法を使うことはおすすめしません。なぜなら、既に $arr に何らかの値 (リクエスト変数からの文字列など) が入っている場合にはその値がそのまま残り、 [] が実際には 文字列アクセス演算子 を表してしまうからです。 変数を初期化するときには、直接代入するほうがよいでしょう。

投稿2019/01/17 06:40

maisumakun

総合スコア145121

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

pgm_bakabon

2019/01/17 07:16

ご回答ありがとうございます。 PHPマニュアルを読むという初歩的なことを忘れておりました。 また、記述部分を抜粋していただきありがとうございます。 レビュー指摘は、 ・変数を利用する前に初期化してほしい ・if(empty($dataIds))$dataIds = array(); というロジックが入ることで可読性が悪くなる という言葉で伝えることにしました。
guest

0

その直しは、質問者様のチームにおけるコーディング規約ではないでしょうか。規約を明確化なさったほうがいいと思います。

私が直すべきと思うのは、

php

1$ids = getIds(); 2$datas = getDatas(); 3 4$mergeIds = array_merge($ids, array_column($datas, 'id'));

です。

なお、こういうコードができあがる背景には私は、

php

1$ids = getIds(); 2$datas = getDatas(); 3 4// $datasが空でforeachに入らないケースがあります 5foreach ($datas as $key => $val) { 6 $dataIds[] = $val['id']; 7} 8$mergeIds = array_merge($ids, $dataIds);

があり、undefined variable: dataIdsが発生したのではないかと推測しています。その解決としては$dataIds ?? []ではないかと思います。

投稿2019/01/17 07:15

papinianus

総合スコア12705

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

あなたの回答

tips

太字

斜体

打ち消し線

見出し

引用テキストの挿入

コードの挿入

リンクの挿入

リストの挿入

番号リストの挿入

表の挿入

水平線の挿入

プレビュー

15分調べてもわからないことは
teratailで質問しよう!

ただいまの回答率
85.50%

質問をまとめることで
思考を整理して素早く解決

テンプレート機能で
簡単に質問をまとめる

質問する

関連した質問