CODEWITHSUNDEEP
phpcoding-stylepdo
Paul S.
Paul S.

Reputation: 1537

Working with dynamic prepared statements in PDO

Sometimes depending on which user type if viewing my page, I need to add in a JOIN, or even just limit the results. Is there a cleaner way of going about it? Should I have separate statements for each type of request instead? What is more "proper"?

Here is what my code ends up looking like:

// Prepare statement
$stmt = $this->db->prepare('
    SELECT *
    FROM    Documents
    LEFT JOIN Notes ON ID = D_ID
    '.($user_id ? "INNER JOIN Users ON UID = ID AND UID = :userid" : '')."
    ". ($limit ? 'LIMIT :offset, :limit' : '')
);

// Bind optional paramaters
if ($user_id) $stmt->bindParam(':userid', $user_id, DB::PARAM_INT);

if ($limit)
{
    $stmt->bindParam(':offset', $limit[0], DB::PARAM_INT);
    $stmt->bindParam(':limit', $limit[1], DB::PARAM_INT);
}

Upvotes: 1

Views: 407

Answers (2)

Dyin
Dyin

Reputation: 5366

I'd create separate (protected) functions, those return a prepared statement that only needs to be executed.

/**
* @returns PDOStatement
*/
protected function prepareStatementForCase1(PDO $dbObject,Object $dataToBind){...}
/**
* @returns PDOStatement
*/
protected function prepareStatementForCase2(PDO $dbObject,Object $dataToBind){...}

Then, I would decide outside, which one has to be called. You can rebuild, maintain and read the code more easily.

Example:

class Document{
  protected $dbObject;

  public function __construct(PDO $dbObject){
    $this->dbObject=$dbObject;
  }
  public function doQuery($paramOne,$paramTwo,...){
    $logicalFormulaOne=...; // logical expression here with parameters
    $logicalFormulaTwo=...; // logical expression here with parameters
    if($logicalForumlaOne){
      $dbStatement=$this->prepareStatementForCase1($dataToBind);
    }else if($logicalFormuleTwo){
      $dbStatement=$this->prepareStatementForCase2($dataToBind);
    }
    $dbResult=$dbStatement->execute();
  }
  protected function prepareStatementForCase1(Object $dataToBind){
    $dbStatement=$this->dbObject->prepare("query string");
    $dbStatement->bindParam(...);
    return $dbStatement;
  }
}

But I would not advice this, when your PDOResult object represents different type of database tuples, or when you return more rows in one of the cases.

What I usually do is that I create a class which represents (in your example) a Document. Only one. I can insert, delete, select, modify by its fields, and handle one item. When I need to (for example) fetch more of them, I create a new class, e.g. DocumentList, which handles a collection of documents. This class would give me an array of Document objects when it fetches more of them.

Upvotes: 0

dqhendricks
dqhendricks

Reputation: 19251

Maybe just wrap the insert strings into their own methods for clarity, like getUserInsertString($user_id), and try to make your quote use more consistent.

Also, are you testing whether $user_id and $limit are defined just by going if ($user_id)? If so, if you had error reporting turned to all, you would get a bunch of undefined variable warnings. You may want to consider using if (isset($user_id)) instead.

Upvotes: 1

Related Questions