Creating a PHP PDO database class, trouble with the OOP

Question

this is my current Database class:

class Database {
 private $db;
 function Connect() {
 $db_host = "localhost";
 $db_name = "database1";
 $db_user = "root";
 $db_pass = "root";
 try {
 $this->db = new PDO("mysql:host=" . $db_host . ";dbname=" . $db_name, $db_user, $db_pass);
 } catch(PDOException $e) {
 die($e);
 }
 }
 public function getColumn($tableName, $unknownColumnName, $columnOneName, $columnOneValue, $columnTwoName = "1", $columnTwoValue = "1") {
 $stmt = $this->db->query("SELECT $tableName FROM $unknownColumnName WHERE $columnOneName='$columnOneValue' AND $columnTwoName='$columnTwoValue'");
 $results = $stmt->fetchAll(PDO::FETCH_ASSOC);
 return $results[0][$unknownColumnName];
 }
}

I'm trying to run it using the following code:

$db = new Database();
$db->Connect();
echo $db->getColumn("Sessions", "token", "uid", 1);

And i get the following error:

PHP Fatal error: Call to a member function fetchAll() on a non-object in /Users/RETRACTED/RETRACTED/root/includes/Database.php on line 19

Any idea what's up? Thanks

• Austin Brunkhorst: On a side note, I would make use of PDO's prepared statements by binding the variables to the query rather than including them directly. For example, ->prepare('SELECT ? FROM ? WHERE ...')->execute(func_get_args());.
• User2013: Thanks Austin, i'll definitely use prepared statements in the future.
• Phil: @AustinBrunkhorst You can't use parameter binding for DB identifiers (table / column names), only values
• Your Common Sense: I kinda like these two upvotes on @AustinBrunkhorst's comment. Keeps my misanthropy up in shape.

Answer 1

1. This function is prone to SQL injection.
2. This function won't let you get a column using even simplest OR condition.
3. This function makes unreadable gibberish out of almost natural English of SQL language.

Look, you even spoiled yourself writing this very function. How do you suppose it to be used for the every day coding? As a matter of fact, this function makes your experience harder than with raw PDO - you have to learn all the new syntax, numerous exceptions and last-minute corrections.

Please, turn back to raw PDO!

Let me show you the right way

public function getColumn($sql, $params)
{
 $stmt = $this->db->prepare($sql);
 $stmt->execute($params);
 return $stmt->fetchColumn();
}

used like this

echo $db->getColumn("SELECT token FROM Sessions WHERE uid = ?", array(1));

This way you'll be able to use the full power of SQL not limited to a silly subset, as well as security of prepared statements, yet keep your code comprehensible.
While calling it still in one line - which was your initial (and extremely proper!) intention.

• User2013: Thanks, needless to say i'm brand new to PDO and despite the tone am appreciative of the lesson you taught.
• hugo der hungrige: -1 Sorry, but using this kind of emphasizes combined with this kind of cocky attitude is simply unnecessary. We all have had to start at some point and it's very likely that you yourself will have to learn new things too.
• Super Cat: You're not really using the full power of SQL if you limit yourself to utilizing class functions that only follow one fetch protocol. It may be better to return $stmt before fetching, and then append whatever fetch function to the returned value. That would also allow you to create a function that would work for non-SELECT queries too. If anything, its purpose is just to avoid having to rewrite prepare() and execute().

Answer 2

it means your $stmt variable is not returning a PDOStatement object. your query is failing since PDO::query either returns a PDOStatement or False on error.

Answer 3

Use fetch instead of fetchAll..that will be easy in your case

$results = $stmt->fetchAll(PDO::FETCH_ASSOC);
return $results[0][$unknownColumnName];

It will be

$results = $stmt->fetch(PDO::FETCH_ASSOC);
return $results[$unknownColumnName];