2016-08-07 23 views
0

私は安全な方法でデータベースからデータを取得するたびに、この長い乱雑なコードを繰り返すことを避けるための考え方はありますか?php oop prepare文

public function test($param) { 
    $sql = "SELECT * FROM users WHERE id = :id AND item_id :item_id"; 
    $this->_query = $this->_db->pdo()->prepare($sql); 
    $this->_query->bindValue(':id', $param); 
    $this->_query->bindValue(':item_id', $parmas); 
    $this->_query->execute(); 
    $this->_results = $this->_query->fetchAll(PDO::FETCH_OBJ); 
} 

は、私はまだないクラスの構造関数で定義されている。この

public function query($sql, $params = array()) { 
    $this->_error = false; 
    if($this->_query = $this->_pdo->prepare($sql)) { 
     if(count($params)) { 
      //outside of the loop 
      $x = 1; 
      foreach($params as $param) { 
       $this->_query->bindValue($x, $param); 
       $x++; 
      } 
     } 
     if($this->_query->execute()) { 
      $this->_results = $this->_query->fetchAll(PDO::FETCH_OBJ); 
      $this->_count = $this->_query->rowCount(); 
     } else { 
      $this->_error = true; 
     } 
    } 
    return $this; 
} 

PDOのような単純なもののために周りの混乱に良い方法を作成しました。

//ここに詳細を追加するとこれが何をしようとしているのですか?

public function example($table, $params = array(), $extn = array(), $values = array()) { 
    $x = ''; 
    $y = 0; 
    foreach($params as $param) { 
     $x .= $param; 
     if($y < count($params)) { 
      $x .= $extn[$y]; 
     } 
     $y++; 
    } 
    $sql = "SELECT * FROM {$table} WHERE {$x}"; 
    $this->_query = $this->_pdo->prepare($sql); 
    foreach($values as $value) { 
     $this->_query->bindValue($value); 
    } 
    $this->_query->execute(); 
    $this->_results = $this->_query->fetchAll(PDO::FETCH_OBJ); 
} 

が、イムこのエラー警告取得:PDOStatementに:: bindValue()は、少なくとも2つのパラメータ、ダウン

$test = new Example(); 
$test->example('users', array('id = :id', 'username = :username', 'id = :username', 'username = :id'), array(' AND ', ' OR ', ' AND '), array("':id', 4", "':username', 'alex'")); 

以下のこのコードの 任意の提案は私には参考になる見込ん!

+1

具体的な問題を明確にしたり、詳細を追加して必要なものを正確に強調してください。現在書かれているとおり、あなたが求めていることを正確に伝えるのは難しいです。この質問を明らかにするには、[How to Ask](http://stackoverflow.com/help/how-to-ask)ページを参照してください。 –

+1

関連する配列、つまり$ params = '[id '=> $ id、' item_id '=> $ item_id]'の中にパラメータを置いて、それを直接execute文で使用することができます。すなわち、 '' $ this - > _ query-> execute($ params); 'それから、通常どおりにフェッチしてください。 PDOは自動的に必要なバインディングをすべて実行し、必要がないようにします。 –

答えて

1

あなたのアプローチでは、厳しいものではありませんが、多数のものがあります。あなたの問題のほとんどは、渡された$valuesが文字化けしていることです。 php-を参照してください参考のため

$values = array(':id' => 4, ':username' => 'alex') 

:私はexampleでbindValuesは()のようなものに見えることを期待する

example()に与えられた

foreach ($values as $param => $value) { 
    $this->_query->bindValue($param, $value); 
} 

したがって$valuesは次のようになりますdocs on PDOStatement::bindValue()

以外:

あなただけのクエリに変数を渡す$table

$sql = "SELECT * FROM {$table} WHERE {$x}"; 

これは安全ではありません!クエリに渡される値(これはわかりやすい)をより心配していますが、ここにはまだ脆弱性が存在します。

あなたのクラスdatabase店舗クエリーとクラス変数でその結果。これは不要です(データベースエンジンにはクエリキャッシュがあります。これらのファイルをキャッシュに保存する場合は、データベースをクラスcached_databaseにラップするなど)。クエリ/結果が誤って再利用されたり混ざり合ったりするとバグが発生する可能性があります。

誤ったパラメータを与えることによってクエリを混乱させる方法はたくさんあります。ほとんどすべてが配列なので、そこに配置する値を特定するのは難しいです。これにより、全体の設定が非常に壊れやすくなります。適切な数の$paramsとがexampleに渡されているため、将来的にはほとんど問題が発生します。そこで何が起こるのか把握するのが難しいだけでなく、INまたはBETWEENのクエリなど、今後使用する可能性のある機能がほとんどないため(ほとんどの場合)数行のコードを書くのは安全かもしれませんが、数週間使用しないと、何が起こり、どのように使用するのかを理解しようとするでしょう。私を信じて、私たちは皆そこにいました。;)

少し繰り返して見えるが、他の人があなたを引き継いだときや将来的にあなたを助けるときに、わかりやすいPDOを繰り返していると思います。

類似のデータを選択するなどの一般的なタスクを簡素化する必要があると感じる場合は、代わりにDoctrineのようなORMを考慮する必要があります。 Doctrine DBALだけでも強力な助けを得ることができます。SQL Query Builder

実際にコードを保存して、他のライブラリを使用したくない場合は、ベビーステップを使用してコードを単純化してみてください。小さな部分が繰り返されるときはいつも全く同じ方法で私的な方法に入れてください。たとえば、最初の例では次の3行を使用してこれを行うことができます。

$ this - > _ query-> execute(); $ this - > _ results = $ this - > _ query-> fetchAll(PDO :: FETCH_OBJ); return $ this - > _ results;

あなたの最初のコードスニペットください:それはですので、これは、あなたのアプローチよりもはるかに安全である

/* 
* Don't just call it "database", that's way to generic. 
* Try to give it an explicit name, for example UserRepository 
*/ 
class UserRepository 
{ 
    public function getUsersByIdAndItemId($id, $itemId) { 
     $sql = "SELECT * FROM users WHERE id = :id AND item_id :item_id"; 
     $query = $this->getPreparedQuery($sql); 
     // No need to simplify this: 
     $query->bindValue(':id', $id); 
     $query->bindValue('item_id', $itemId); 

     return $this->executeQuery($statement); 
    } 

    public function getAllUsers() 
    { 
     $sql = "SELECT * FROM users"; 
     $query = $this->getPreparedQuery($sql); 

     return $this->executeQuery($query); 
    } 

    private function getPreparedQuery($sqlQueryString) 
    { 
     /* 
     * No "$this->_query" 
     * You don't have to reuse it! Worst case scenario a diffeent 
     * method accidentally reuses the query and you get a PDOException. 
     */ 
     return $this->_db->pdo()->prepare($sqlQueryString); 
    } 

    private function executeQuery(\PDOStatement $query) 
    { 
     $statement->execute(); 

     /** 
     * Again no need to store this in a class variable. 
     * Worst case scenario you end up with a dirty state 
     * or mixed up data 
     */ 
     return $statement->fetchAll(PDO::FETCH_OBJ); 
    } 
} 

:あなたはちょうどあなたが小さなプライベートメソッドに頻繁に繰り返されるビットを抽出して、それを簡素化することができ

class database { 
    $_results = null; 
    /*** 
    @param array $param should be an array of two elements 
    ***/ 
    public function test($param = []) { 
     $sql = "SELECT * FROM users WHERE id = :id AND item_id :item_id"; 
     $this->_query = $this->_db->pdo()->prepare($sql); 
     $this->_query->bindValue(':id', $param); 
     $this->_query->bindValue(':item_id', $param); 
     $this->_query->execute(); 
     $this->_results = $this->_query->fetchAll(PDO::FETCH_OBJ); 
     return $this->_results; 
    } 
} 

をシンプルでクリアです。あなたの入力は常にセキュリティで保護されており、いつでもUserRepsitory::findUsersByIdAndItemId()を使用すると、間違って多くのパラメータを送信したり、間違った名前を付けたり、SQLクエリを駄目にすることはありません。あなたはちょうどあなたが必要とする値を正確に渡します。時には "自分自身を繰り返さないでください"に違反することは、コードがより安全で理解しやすくなるなら、大丈夫です。

+0

きれいに見えて、私が望んでいたものと同じことをします!提案ありがとう。 –