php if内でDB接続→SQL実行後に文字列でメッセージを表示させたいがエラーとなる

Question

DB接続後、update SQLで更新した後に「更新に成功しました。」とメッセージを表示させていのですがエラーとなります、

具体的に
if文の中に文字列を入れようと「echo '更新に成功しました。';」と入れるとエラーとなってしまいます。

お助けお願いします。

###問題ファイル

<?php

if(!empty($_REQUEST["post"])){
	$pdo=new PDO('mysql:host=localhost;dbname=shop;charset=utf8', 
	'staff', 'password');
	$sql=$pdo->prepare('update product set name=?, price=? where id=?');

	$sql->execute(
		[htmlspecialchars($_REQUEST['name']), 
		$_REQUEST['price'], $_REQUEST['id']]
) 

	echo '更新に成功しました。';　//ここでエラー（syntax error, unexpected 'echo' (T_ECHO)）


	}

else {
	echo '更新に失敗しました。';
}

Answer 1

    $sql->execute(
        [htmlspecialchars($_REQUEST['name']), 
        $_REQUEST['price'], $_REQUEST['id']]
) 

↓

    $sql->execute(
        [htmlspecialchars($_REQUEST['name']), 
        $_REQUEST['price'], $_REQUEST['id']]
    );

Comments

[Ms.yy]

ありがとうございます。
エラーは解消されました。

文法ミスとは失態です。汗

Answer 2

if(!empty($_REQUEST["post"])) の else は、POSTデータが空なのであって、更新に失敗したわけではないですよね？
更新の失敗は失敗で、try catchで失敗原因のエラーを吐き出させるべきです。

あと個人的に（これは趣味嗜好の範囲ですが）if文やforeach文は｛｝ではなく以下のほうが読みやすいと思います。
（try catchと併用すると特に）

if(!empty($_REQUEST["post"])): //{と同義
  try{
    $pdo=new PDO('mysql:host=localhost;dbname=shop;charset=utf8','staff','password');
    $sql=$pdo->prepare('update product set name=?, price=? where id=?');
    $sql->execute(
            [htmlspecialchars($_REQUEST['name']), 
            $_REQUEST['price'], $_REQUEST['id']]
          ); 
    echo '更新に成功しました。';//ここまでたどり着けば更新に成功している
  }catch (PDOException $e){//更新データを受けているにもかかわらず何らかのエラーにより失敗した
    header('Content-Type: text/plain; charset=UTF-8', true, 500);
    echo '更新に失敗しました。';
    echo($e->getMessage());//エラーを出力
    exit;//以後の処理を停止
  }
else: //{と同義
  echo 'POSTデータが空です。';
endif; //}と同義

Comments

[Ms.yy]

すみません「if(!empty($_REQUEST["post"]))」この辺こんがらがってきたのでここの部分勉強し直します汗

こちらの方がわかりやすいですね！
そんな書き方知りませんでしたよ＾＾

[Take_it]

いやまぁ、if文の書き方は蛇足（あくまでも見やすさと好みの問題なので）であって、そもそもPOSTデータが空の場合と更新失敗の区別のほうは大丈夫ですか？
質問中のコードだとエラーもcatchできていませんし、そっちのほうがif文の書き方よりよほど重要です。

[Ms.yy]

空の場合と更新の区別は分からずです汗

上記のコード文ですと、フォームの送信後そのpostが空でない場合if(!empty($_REQUEST["post"]))〜の中身が実行されて

空だった場合(URLにダイレクトにアクセス)はelse文を想定してます。

[Take_it]

＞上記のコード文ですと、フォームの送信後そのpostが空でない場合if(!empty($_REQUEST["post"]))〜の中身が実行されて
＞空だった場合(URLにダイレクトにアクセス)はelse文を想定してます。

そこはそれでまぁOKとして、if(!empty($_REQUEST["post"]))〜の中身で何らかのエラーが起きて正常に更新できないのと、空だった場合(else)とを区別しておく必要があります。
そのため、SQLの実行を用いて、エラーの排出と処理の中断をする必要があるのです。

質問者さんの最初のコードをただ構文エラー訂正しただけ（セミコロン追記のベストアンサー例）だと、

$_REQUEST["post"]が空でなかった場合
　更新に成功しようが失敗しようが、「更新に成功しました」と表示され

$_REQUEST["post"]が空だった場合のみ
　「更新できませんでした」

となります。
$_REQUEST["post"]がたとえ空ではなくても例えばデータ型が不一致だったり、キーの重複（duplicate）だったり、データベース側の不具合等で例外処理（正常に更新できずエラーになる）場合でも、「更新に成功しました」と表示されてしまいます（本当は成功していなくても）

成功しなかった場合に成功しなかった事実とその原因（エラーメッセージ）を出力しないと、後々困ると思いますよ。そのためのtry-catchです。

[Ms.yy]

なるほどです、
区別するためにtry　catchを入れた方がいいのですね。
このtry catchいらないでしょって思ってたので汗
勉強しなきゃです。

if文の時に設定しておくものなんですね。

[Take_it]

>if文の時に設定しておくものなんですね。
上記の私の説明がわかりにくかったようですが、違います。

データベース使って何かするときは必ずtry catchでPDOExceptionを拾って、エラー出力をすべきです。
そうしないと、今回の質問を例に取れば、SQL側で何らかのエラーが生じても、エラーが生じたことすらわからず次の処理が続行してしまいます。
「成功しました」と画面に出ているのに実は成功していない。登録に成功したはずのデータが存在しない（成功メッセージは出たのに）、更新に成功したはずのデータが古いまま（成功メッセージは出たのに）、削除に成功したはずのデータが存在する（成功メッセージは出たのに）・・・という、わけのわからん状態になるわけです。
きちんとtry catchでPDOExceptionを拾ってエラーを表示することによってはじめて、「成功」「不成功」を識別し、不成功の場合のエラーメッセージを表示することで原因の特定・改修が可能になるのです。

if文を使うかどうかは 関　係　が　あ　り　ま　せ　ん　！

[退会済みユーザー]

@Take_it さん
try - catch の説明するなら、PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION
の説明も必要かと思いますよ。
https://www.php.net/manual/ja/pdo.setattribute.php

[Take_it]

Kosuke_Shibuya さん
あー。ご指摘の通りです。抜けてました。

＞$pdo=new PDO('mysql:host=localhost;dbname=shop;charset=utf8','staff','password');

の部分で、

$pdo=new PDO(
'mysql:host=localhost;dbname=shop;charset=utf8','staff','password',
array(
PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION,
PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC,
)
);

とでもしておくべきですね。

[Ms.yy]

失礼しました、DBの時にtry catch文なんですね。＾＾

そして、DB接続時に「array(
PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION,
PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC,
)」これも必要と・・・

ここら辺調べ直します！