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

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());. — Austin Brunkhorst
– Austin Brunkhorst, Commented Aug 20, 2013 at 3:35
Thanks Austin, i'll definitely use prepared statements in the future. — User2013
– User2013, Commented Aug 20, 2013 at 4:36
@AustinBrunkhorst You can't use parameter binding for DB identifiers (table / column names), only values — Phil
– Phil, Commented Aug 20, 2013 at 5:43
I kinda like these two upvotes on @AustinBrunkhorst's comment. Keeps my misanthropy up in shape. — Your Common Sense
– Your Common Sense, Commented Aug 20, 2013 at 7:50

Your Common Sense · Accepted Answer · 2016-03-23 15:11:39Z

5

This function is prone to SQL injection.
This function won't let you get a column using even simplest OR condition.
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.

edited Mar 23, 2016 at 15:11

answered Aug 20, 2013 at 5:33

Your Common Sense

158k42 gold badges226 silver badges373 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

User2013 Over a year ago

Thanks, needless to say i'm brand new to PDO and despite the tone am appreciative of the lesson you taught.

hugo der hungrige Over a year ago

-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 Over a year ago

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().

roninblade · Accepted Answer · 2013-08-20 03:33:12Z

1

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.

answered Aug 20, 2013 at 3:33

roninblade

1,91215 silver badges15 bronze badges

Comments

William Isted · Accepted Answer · 2016-03-23 14:59:39Z

0

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

edited Mar 23, 2016 at 14:59

William Isted

12.4k4 gold badges34 silver badges47 bronze badges

answered May 12, 2015 at 9:53

Hytool

1,3781 gold badge8 silver badges22 bronze badges

Collectives™ on Stack Overflow

Creating a PHP PDO database class, trouble with the OOP

3 Answers 3

3 Comments

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

3 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related