PHP framework building: MySQL Connection and query class

I am building a PHP framework and would like to get some feedback on a few different sections of the project so far. I consider myself still a neophyte in PHP so I would like to ask if I'm going about completing these different tasks in an efficient and or correct way.

This section is of the MySQL Connection class I have created for it. So for I have created the basic operations for connecting and querying the database. I have posted below the class code and a script showing how it can be use.

I would like to get feedback on if this is the proper way to create this type of class and an efficient way to complete database querying tasks.

MySQL Connection Class

```
class MysqlConnection{
//Connetion login
private $host;
private $user;
private $password;
private $database;
private $connection;
//Query being used
public $query;
/**
* @param String $host - server host
* @param String $user - username
* @param String $database - database name
* @param String $password - username password
* @return Null
*/
public function __construct($host,$user,$password,$database){
if(isset($host,$user,$password,$database)){
//If parameter supplied
$this->host = $host;
$this->user = $user;
$this->password = $password;
$this->database = $database;
}else{
return false;
}

}
/**
* Send query to database
*
* @param String $query - query statment
*
* Note: This allow a query to the DB if
* the class is going to be used withing a fuction
* or method
*/
public function Query($query){
if(isset($this->connection)){
if(($this->query = @$this->connection->query($query))){
return $this->query;
}
return false;

}else{
return false;
}

}

public function LastId(){
return $t

Use early returns to simplify if statements

This code from the insert function:

if(isset($this->connection)){
        if(($this->query = @$this->connection->query($query))){
            return $this->query;  
        }
            return false;

    }else{
        return false;
    }

Can be rewritten like this:

if(!isset($this->connection)){
        return false;
    }
    if(($this->query = @$this->connection->query($query))){
        return $this->query;  
    }
    return false;

You can use this principle in other functions as well (for example for the isset check in update, delete and select).

In this case, I would make the setting of $this->query more explicit:

if(!isset($this->connection)){
        return false;
    }
    $this->query = @$this->connection->query($query);
    return $this->query;

You don't need a separate command to return false, as query returns false on failure.

Further if simplifications

This from your select function:

if(($result = $this->connection->query($sql))){
            if($result->num_rows > 0){
                return ($this->query = $result);
            }
            return false;
        }
        return false;

can be collapsed to one if statement:

$result = $this->connection->query($sql);
        if($result && $result->num_rows > 0){
            return ($this->query = $result);
        }
        return false;

And this from your delete function:

if($this->connection->affected_rows > 0){
            return true;
        }
        return false;

can be rewritten like this:

return $this->connection->affected_rows > 0;

Result function

Do you expect a use case where you need to pass a parameter to Result? If not, I would remove it, it just complicates your code.

Either way, right now, if I pass null (or nothing), and $this->query is empty, fetch_assoc will be called on null.

update function

Right now, multiple updates can be executed with one call of update. If one fails, the rest are not executed. But the previously executed once are not rolled back either.

And the caller doesn't know which statements got executed and which didn't. I think that this might cause problems in the future.

SQL injection

addslashes does not adequately defend against SQL injections (see for example here, here, or here).

See also this answer where I link to more resources about SQL Injections and prevention of it.

Also, it might be confusing for a future user of your class that some values are escaped while others are not (and thus the escaping is the task of the caller). $table, $columns, $column, $where, and of course $query are not escaped, while $value is.

Style Guidelines

• generally, function names should start with a lower-case letter (you can even see it here in the code highlighter: the upper-case functions are rendered the same as the class, while the lower-case functions are not).

• a minor point: try to be consistent with your curly brackets and the spaces around them (){ vs ) {).

• be consistent with your indentation.

• be consistent with string concatenation (FROM $table WHERE $where vs FROM ".$table." WHERE ".$where).

• be consistent with your single- and double-quotes ("num, text, other" vs 'dummy').

General Approach

For me, your example calls:

insert(array('num'=>4,'text'=>'New insert','other'=>'This is a text'),'dummy')
select("num, text, other",'dummy','num = 4')

are a bit hard to read. You could instead implement a query builder to make them look something like this:

new QueryBuilder().select("num", "other").from("dummy").where("num = ?", 4)

Note also that the 4 is separate from the sql syntax, thus making it easier to use prepared statements when executing such a query.

For ideas for a querybuilder interface, you could look at existing once, such as FluentPDO or the QueryBuilder of Doctrine. You could also take a look at the source code of FluentPDO to get some ideas (it's just a couple of files).

StackExchange Code Review Q#63593, answer score: 2