PDO (MySQL) connection and query class, safety and logic

Question

For the last few day I tried to figure out how to work with MySQL and PDO. Although I've tried to read a lot about the subject, there are still a lot of things I don't understand.

Because of this lack of knowledge, I can't really judge this code (or other example code on-line) on its safety and logic, and therefore was hoping I could get some feedback on it.

The class makes a connection and returns a query:

class Connection
{
private $username = "xxxx";
private $password = "xxxx";
private $dsn = "mysql:host=host;dbname=name_db";
private $sql;
private $DBH;
function setSQL($sql){
 $this->sql = $sql; //if $sql is not set it troughs an error at PDOException
}
public function query()
{
 //the connection will get established here if it hasn't been already
 if (is_null($this->DBH))
 try { 
 $this->DBH = new PDO( $this->dsn, $this->username, $this->password );
 $this->DBH->setAttribute( PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION );
 //query 
 $result = $this->DBH->prepare($this->sql); 
 $result->execute();
 return $result->fetch(PDO::FETCH_ASSOC); 
 } catch(PDOException $e) { 
 echo "I'm afraid I can't do that1."; 
 //file_put_contents('PDOErrors.txt', $e->getMessage(), FILE_APPEND); 
 }
}
//clear connection and variables
function __destruct(){
 $this->DBH = null;
}
}

Using the class:

$sql = "SELECT * from stock WHERE id = 302"; 
$test = new Connection();
$test->setSQL($sql);
echo $test->query();

Answer 1

1) When using PDO, you want to get the benefits of automatic parameter escaping. By wrapping your PDO class you are limiting it's functionality. Check out this question for a better example:

https://stackoverflow.com/questions/6366661/php-pdo-parameterised-query-for-mysql

2) Every time you run a query you are making a new PDO instance. It might be better to have an application resource pool that you can call to get a preconfigured db handle.

3) You wrote some sql to be able to fetch a stock by ID, that should probably be functionality that is reusable.

combining 1 & 2 & 3 .. code would probably look better as something like this:

class ApplicationResourcePool 
{
 static var $_dbHandle;
 private static $_dbConfig = array(
 'dsn' => '',
 'username' => '',
 'password' => '',
 );
 public static getDbHandle(){
 if(self::$_dbHandle == null){
 self::$_dbHandle = new PDO(
 self::$_dbConfig['dsn'] , 
 self::$_dbConfig['username'], 
 self::$_dbConfig['password'] 
 );
 }
 return self::$_dbHandle;
 }
}
class StockMapper
{
 protected $_dbh; 
 public __construct($dbh = null)
 {
 if($dbh == null){
 $this->_dbh = ApplicationResourcePool::getDbHandle();
 } else {
 $this->_dbh = $dbh;
 }
 }
 public getStockById($stockId){
 $sth=$this->_dbh->prepare("SELECT * from stock WHERE id = :stockId"); 
 $sth->bindParam(":stockId", $stockId);
 $sth->execute();
 $result = $sth->fetch(PDO::FETCH_ASSOC);
 return $result[0];
 }
}

your codeBlock then becomes:

$stockMapper = new StockMapper();
$stockData = $stockMapper->getStockById('302');

• Rob: perfect!, that seems like a more logic and better way to handle this. Would you have any advice how to handle the PDOException in this example? Would you add it as a function('s) to getStockById?
• Zak: I'd probably log the exception in a logger and in turn throw another exception that my outer code could catch. Your outer code shouldn't need to know that the stock mapper is internally using a DB or PDO as it's data storage/retrieval.
• Zak: The one other thing I would probably do is create a "Stock" class that my mapper could populate with the data array from the database. Then I get a first class object instead of a PHP array as a return value.

Answer 2

Two issues:

1) I would place the code to open the connection in the constructor.

2) You want to look into parametrized prepared statements.

You want to send a query like:

SELECT * from stock WHERE id = ?

Then send an array of parmeteters:

array(302)

This will prevent SQL injection.

• Rob: I see the logic in there, but how do you deal with the different query's you need? like: SELECT stock.size people.age from sizes LEFT JOIN people ON stock.id = people.id?
• Len: Well you can have queries as complex as you wish but the user parameters will ultimately become ?'s. You can also create a query builder where you can say $test->leftjoin(table) and build up SQL. This will slow you down a bit though if the server is getting a lot of hits.