Is this a secure OO php database connection or a waste of code

Question

So I have been programming for a while and decided to learn about OOP. I made this class for a database connection and I am unsure if this is a waste of code or a good way to do things. I still don't feel like I understand OOP that much but I will get there it's just practice. I guess my aim really was to keep as much of the database connection private to the class as I could and also have the class do all the cleanup like mysqli_close();.

class db {

private $db_user;
private $db_pass;
private $db_host;
private $db_name;
private $link;
private $db_error;

public function escape($string) {

    return mysqli_real_escape_string($this->link, $string);

}

public function query($query) {

    return = mysqli_query($this->link, $query);

}

function __construct() {

    $this->db_error = 'Database Error';
    $this->db_user = 'root';
    $this->db_pass = '';
    $this->db_host = 'localhost';
    $this->db_name = 'test';

    $this->link = mysqli_connect($this->db_host, $this->db_user, $this->db_pass) or die($this->db_error);
    mysqli_select_db($this->link, $this->db_name) or die($this->db_error);

}

function __destruct() {

    mysqli_close($this->link);

}

}

Edit: Thanks for the answers I am going to learn PDO.

`mysqli_free($result);` will never get run as it is placed **after** the `return` statement. — Aiias, Jun 27 '13 at 19:38
Stop wrapping native extensions. Also note `mysqli_free($result);` will never execute. — Jason McCreary, Jun 27 '13 at 19:38
I think this question is more suitable for [Code Review](http://codereview.stackexchange.com/). — juan.facorro, Jun 27 '13 at 19:41
You might find [this post](http://stackoverflow.com/a/11369679/727208) somewhat useful. It also can be applied to mysqli. — tereško, Jun 27 '13 at 19:57

score 3 · Accepted Answer · edited Jun 27 '13 at 19:41

3

It's not wrong per se, except inasmuch as it fails to support prepared statements and is thus open to exploitation, but neither is it a useful thing on which to spend your time; PHP's PDO class is the wheel for which you are searching.

edited Jun 27 '13 at 19:41

crush

16,713
9
59
100

answered Jun 27 '13 at 19:39

Aaron Miller

3,692
1
19
26

score 0 · Answer 2 · answered Jun 27 '13 at 20:03

I think its a good idea. It's better to encapsulate all database functionality in a class.

Some suggestions:

send the connection parameters in the constructor. So, you can reuse it easily in other projects.
Use prepared statements, so escaping string becomes redundant
mysqli_query can fail also and you need to handle it.

score 0 · Answer 3 · answered Jun 27 '13 at 20:44

I would like to comment on your question but do not have enough reputation so I will just point out my thoughts as an answer.

What you are essentially doing is trying to write a wrapper - which is absolutely fine. But as others have pointed out, it would be better to wrap the PDO functions instead, as they offer prepared statements to handle any SQL injection attempts and are fully supported for the future.

Another problem I see is the fact that you are initializing and storing your MySQL login credentials inside of the object. Since they are now in memory, this information is exposed and can be retrieved.

I would suggest that you store your MySQL login information in a configuration file e.g (.ini) and read it in when you call the mysqli_connect() function. Don't store this data in class member variables.

If you decide to store the login info as local variables (perhaps to make the code clearer with meaningful variable names) be sure to null them out after calling mysqli_connect().

Is this a secure OO php database connection or a waste of code

3 Answers3