CODEWITHSUNDEEP
phpooppdo
User2013
User2013

Reputation: 409

Creating a PHP PDO database class, trouble with the OOP

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

Upvotes: 3

Views: 11580

Answers (3)

Your Common Sense
Your Common Sense

Reputation: 157870

  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.

Upvotes: 5

Hytool
Hytool

Reputation: 1368

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];

Upvotes: 0

roninblade
roninblade

Reputation: 1892

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.

Upvotes: 1

Related Questions