Secure Functions in a Database Class

I am trying to solve as many issues as possible with my Database Class and bind statements as far as possible without actually doing it in the front-end. The goal is to do all of the heaving lifting for PHP Developers and Web Designers as possible to make their job even more easier than other PHP frameworks.

The Database class is my main worry to make sure that I keep security with simplexity.

class Database {
// Instance of the Database and Error
private $db;
private $error;
private $stmt;

    public  function  __construct ()  {
    //  Init the database connection
    //  Sets the connection type
    $host = 'mysql:host' . $this->_hostname ';dbname' . $this->_database;

    //  Sets the connection options
    $options = array(
                     PDO :: ATTR_PERSISTENT  =>  TRUE ,
                     PDO::ATTR_ERRMODE    => PDO:ERRMODE_EXCEPTION
                    );

    try {
        //  Generates the Database connection of our class
        this->db = new PDO($host, $this->_username, $this->_password, $options);
    }
    catch(PDOException $exception) {
        //  Catches all errors from the Database connection
        $this->error = $exception->getMessage();
    }
}

The instance of the Database is defined and called in __construct() to allow it to be called globally in other classes.

For queries to be done, I managed a small (secure?) work around in the form of functions

public function execute() {
    //  Executes our SQL query 
    return $this->stmt->execute();
}

public function results() {
    //  Returns all of the results from the table
    $this->execute();
    return $this->stmt->fetchAll(PDO::FETCH_ASSOC);
}

public  function  result ()  {
    //  Returns one of the results from the table
    $this->execute();
    return $this->stmt->fetch(PDO::FETCH_ASSOC);
}

With my knowledge, this is the most secure route of handling PDO and information unless I am wrong?

The other way, which seems to be more dangorious and common is

This may sound harsh, I meant to be friendly though :)

It's bad/unsafe/vulnerable. This type of situation comes around a lot, which is sort of understandable. However, what is your class really doing? It's essentially pointless to make this class. You've basically just put a class in a class without doing anything different! So actually what you've done is make things less secure.

You mentioned a "more dangerous" option of preparing the query. That would be a false assumption. Preparing queries are one of the largest features of the PDO and MySQLi interfaces. They're far from dangerous if used correctly.

It's hard to tell if what you're really doing is "secure" because the actual SQL handling functions you've left out.

Now that that's out of the way...

To improve your coding, you can:

• Fix indentation

• Pick a coding style and apply it. You want clean code, not messy. Here's Zend's styles, which is one I like to offer because it's easy to follow and apply.

Then you're code will be nice and tidy!

Hows 'bout that PHP now!

Starting from the top:

$host = 'mysql:host' . $this->_hostname ';dbname' . $this->_database;

Where are you getting $_hostname and $_database? Same with $_username and $_password. It's okay to add those in your question next time (just without the actual values!) :)

In you first try...catch you have a typo. The assignment to this->db should really be $this->db. I'm not sure why you didn't receive an error for that.

This

public function execute() {
    return $this->stmt->execute();
}

is a nice try, but what's it's point? It's really not useful at all. That's be another reason home-made DB access classes aren't so great. If you do keep it, is it being called by the caller? If not, then you could make it private at least.

In both results() and result(), you return the associative array (PDO::FETCH_ASSOC) which may not be the best idea. What if you want to change the format of return down the line? There's not one easy way to do this.

Speaking of those two functions, I suggest you changes their names. I almost started to write how you had to of the same function. It's hard to tell them apart is what I'm saying.

So overall, there are many database access classes that come with frameworks that actually do improve the safety and handling of databases, unfortunately it's difficult to replicate your own. Don't be discouraged though! It's not a simple task, Laravel has about 50 different classes + interfaces just to make sure connecting to the database is perfect!

StackExchange Code Review Q#54568, answer score: 5