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

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

質問する

ただいまの
回答率

90.12%

PHP/MySQL PDOを使った複数のSQLを文を実行する処理

解決済

回答 2

投稿 ・編集

  • 評価
  • クリップ 4
  • VIEW 9,210
junkboy

junkboy

score 43

1つ関数内で複数のSQL文を実行する場合の処理なのですが、下記ソースのように2回(実行回数)「execute();」を使用して処理してしまっても、可読性など問題はないでしょうか。

こうすれば可読性が上がったり、短縮してかけるよ、という事があればご教授いただけると、大変助かります。 

function cuteInsert() {
    $dbh = connectDb();
    $dbh->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);

    // 1回目
    $post_id = h($_POST['post_id']);
    $sql = "insert into post_ip(post_id, remote_addr, user_agent, created ) values (:post_id, :remote_addr, :user_agent, now())";
    $stmt = $dbh->prepare($sql);
    $stmt->setFetchMode(PDO::FETCH_ASSOC);
    $stmt->bindValue(':remote_addr', $_SERVER['REMOTE_ADDR'], PDO::PARAM_STR);
    $stmt->bindValue(':user_agent', $_SERVER['HTTP_USER_AGENT'], PDO::PARAM_STR);

    try {
      $stmt->execute();
    } catch (\PDOException $e) {
      throw new \Exception('今日は投稿できません。');
    }

    // 2回目
    $eva = h($_POST['evaluation']);
    if ($eva == "0") {
      $evaluation = 1;
    } else {
      $evaluation = $eva + 1;
    }

    $cute_sql = "update post set evaluation = :evaluation where post_id = :post_id";

    $stmt = $dbh->prepare($cute_sql);
    $stmt->setFetchMode(PDO::FETCH_ASSOC);
    $stmt->bindParam(':post_id', $post_id, PDO::PARAM_INT);
    $stmt->bindParam(':evaluation', $evaluation, PDO::PARAM_INT);
    $stmt->execute();

    return $evaluation;
  }

↓Kosuke_Shibuyaさん、KiyoshiMotokiさんにご指摘いただいた内容を加味し、ソースを修正しました。 

  function cuteInsert() {
    $dbh = connectDb();
    // $post_id = h($_POST['post_id']); から変更
    $post_id = filter_input(INPUT_POST, 'post_id');

    $sql = "insert ignore into post_ip(post_id, remote_addr, user_agent, created ) values (:post_id, :remote_addr, :user_agent, now())";
    $dbh->beginTransaction();
    $stmt = $dbh->prepare($sql);
    $stmt->bindValue(':post_id', $post_id, PDO::PARAM_INT);
    $stmt->bindValue(':remote_addr', $_SERVER['REMOTE_ADDR'], PDO::PARAM_STR);
    $stmt->bindValue(':user_agent', $_SERVER['HTTP_USER_AGENT'], PDO::PARAM_STR);
    $stmt->execute();
    $count = $stmt->rowCount();

    // 登録できた場合
    if ($count == "1") {
      $eva = filter_input(INPUT_POST, 'evaluation');
      if ($eva == "0") {
        $evaluation = 1;
      } else {
        $evaluation = $eva + 1;
      }

      $cute_sql = "update post set evaluation = :evaluation where post_id = :post_id";

      $stmt = $dbh->prepare($cute_sql);
      $stmt->bindValue(':post_id', $post_id, PDO::PARAM_INT);
      $stmt->bindValue(':evaluation', $evaluation, PDO::PARAM_INT);
      $stmt->execute();
      $dbh->commit();
    // 登録できなかった場合
    } else {
      $dbh->rollBack();
    }
  }

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

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

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

    クリップを取り消します

  • 良い質問の評価を上げる

    以下のような質問は評価を上げましょう

    • 質問内容が明確
    • 自分も答えを知りたい
    • 質問者以外のユーザにも役立つ

    評価が高い質問は、TOPページの「注目」タブのフィードに表示されやすくなります。

    質問の評価を上げたことを取り消します

  • 評価を下げられる数の上限に達しました

    評価を下げることができません

    • 1日5回まで評価を下げられます
    • 1日に1ユーザに対して2回まで評価を下げられます

    質問の評価を下げる

    teratailでは下記のような質問を「具体的に困っていることがない質問」、「サイトポリシーに違反する質問」と定義し、推奨していません。

    • プログラミングに関係のない質問
    • やってほしいことだけを記載した丸投げの質問
    • 問題・課題が含まれていない質問
    • 意図的に内容が抹消された質問
    • 広告と受け取られるような投稿

    評価が下がると、TOPページの「アクティブ」「注目」タブのフィードに表示されにくくなります。

    質問の評価を下げたことを取り消します

    この機能は開放されていません

    評価を下げる条件を満たしてません

    評価を下げる理由を選択してください

    詳細な説明はこちら

    上記に当てはまらず、質問内容が明確になっていない質問には「情報の追加・修正依頼」機能からコメントをしてください。

    質問の評価を下げる機能の利用条件

    この機能を利用するためには、以下の事項を行う必要があります。

回答 2 件

link

+3

提示のコードだと、「update」の処理は絶対に行われませんね…
（コードに間違いがあるので…）

下記ソースのように2回(実行回数)「execute();」を使用して処理してしまっても、可読性など問題はないでしょうか

「可読性」を考慮するなら、自分だったら処理は分けますし、グローバルな関数は定義せずに、クラスで実装します。

他に気づいたところでは、

この部分は、別の関数に切り分けする。 
$dbh = connectDb();
$dbh->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);

提示のコードで以下の部分は無意味 
$stmt->setFetchMode(PDO::FETCH_ASSOC);
例外ブロックを変更すべき 
try ~ catch (\PDOException $e) {
}

以下の部分 
$post_id = h($_POST['post_id']);

ここで使われている h() が htmlspecialchars 相当の機能であるなら、使用方法を間違っています。

以下の2つについても、\ の意味を理解せずに使っているのかなと。 
$dbh->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);
catch (\PDOException $e) {

投稿

編集

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2015/11/28 04:35

    PDO の 前につける `\` は、名前空間を意味する記号です。
    http://php.net/manual/ja/language.namespaces.rationale.php

    キャンセル

  • 2015/11/28 04:38

    クラスについて
    同種の関数のまとまりを持たせる意味もあるので、自分は、
    ・DB接続
    ・SELECTの実行
    ・UPDATEの実行
    ・INSERTの実行
    ・DELETEの実行
    というような感じでクラスにまとめています。

    キャンセル

  • 2015/11/30 12:10

    ・'\'について
    調べても辿りつけなかったので、大変助かります。

    ・クラスについて
    なるほど、DBで行う処理内容によって、クラスを作っておられるのですね。
    大変参考になります。

    投稿した内容とは違う質問にも関わらず、ご丁寧に回答いただき、ありがとうございました。

    キャンセル

link

checkベストアンサー

+2

細かい話ですが、
1回目のクエリ実行ではbindValueを、2回目ではbindParamを使用しているのが気になります。
"可読性"という点では、特に理由がなければどちらかに統一した方が良いと思います。


以下、ご質問の内容（行数・可読性）とは若干ズレますが、提示いただいたコードで気になった点を上げさせていただきます。
（Kosuke_Shibuya様の回答と重複する内容もあります）

 レコードの存在確認について

post_ipテーブルに既にレコードが登録されているか否かを、
PDOExceptionが送出されたか否かで判断するのはよろしくありません。

この方法では、DBに障害が発生した場合など、本当に例外的な状況との区別がつかないからです。

MySQLをお使いであれば、insert into ...をinsert ignore into ...に変更し、
PDOStatement::rowCount()メソッドで登録された行数を確認する方が良いです。
http://php.net/manual/ja/pdostatement.rowcount.php

 トランザクションを使用していない

トランザクションを使用していないため、以下のような状況が発生する可能性があります。
  1.  1回目のクエリ実行 -> 成功
  2.  DB障害発生
  3.  2回目のクエリ実行 -> 失敗
  4.  結果、post_ipテーブルにレコードは登録されたが、postテーブルは何も変更されない

1回目と2回目のクエリはトランザクションの中で実行し、クエリの実行に失敗したらロールバックしてやる必要があります。
http://php.net/manual/ja/pdo.begintransaction.php

 $stmt->setFetchMode(PDO::FETCH_ASSOC); は不要

SQL文の実行後に$stmt->fetch()を実行しない場合、$stmt->setFetchMode()を実行する必要はありません。
http://php.net/manual/ja/pdostatement.setfetchmode.php

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2015/11/25 11:29

    細かい点までリンク付きでご指摘いただき、ありがとうございます。
    Kosuke_Shibuyaさんにご指摘いただいた箇所と合わせて、1つ1つ落とし込んでいきたいと思います。

    大変参考になります。
    取り急ぎ、お礼まで。

    キャンセル

  • 2015/11/25 14:53

    ・bindValue、bindParam
    参照している変数の中身が変わると、別の値がバインドされる可能性があるということだったので、bindValueに統一しました。

    ・FETCH_ASSOC
    別の処理で取得結果を配列に格納するために使用しており、それをコピペしていたので削除しました。

    ・レコードの存在確認/トランザクション
    なるほど、'rowCount()'メソッドを使用して登録された行数を確認する方法があるのですね。

    トランザクションについても、ご丁寧にありがとうございます。
    ご指摘頂いた内容でソースを更新いたしましたので、お時間のあるときに意向にそった内容になっているか、ご確認いただけますと幸いです。

    キャンセル

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

  • ただいまの回答率 90.12%
  • 質問をまとめることで、思考を整理して素早く解決
  • テンプレート機能で、簡単に質問をまとめられる
質問する

関連した質問

同じタグがついた質問を見る

SNSアカウントでログイン

teratailアカウントでログイン

思考するエンジニアのためのQ&Aサイト「teratail」について詳しく知る