HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

MySQLi library for handling MySQLi interactions

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
handlinginteractionsforlibrarymysqli

Problem

I wrote this class recently to better structure and handle MySQLi related interactions with PHP and would love to get some feedback.

```
'',
'username' => '',
'password' => '',
'database' => ''
);
public $table = '';

/*
* __construct($databaseConfig = array())
*
* Instantiate simpleMysqli class with optional database config.
*
* @param (databaseConfig) An optional database configuration
* containing host, username, password, database.
*
*/
public function __construct($databaseConfig = array())
{
$this->database = ($databaseConfig) ? $databaseConfig : $this->database;

parent::__construct();
$this->real_connect(
$this->database['host'],
$this->database['username'],
$this->database['password'],
$this->database['database']
);
}

/*
* preparedQuery
*
* Structures, maps and executes a prepared MySQLi query.
*
* @param (mappedQuery) A properly formatted SimpleMYSQLi query.
* @param (mappedParameters) An associative array of parameters
* to map into the mappedQuery.
* @param (returnInsertId) Returns the insert id for insert queries.
* @param (returnAffectedRows) Returns the number of rows affected by
* the executed query.
*
* @return A MySQLi result, or insert id, or affected rows depending
* on what parameter values were passed.
*/
public function preparedQuery($mappedQuery, $mappedParameters = array(), $returnInsertId = 0, $returnAffectedRows = 0)
{
$dynamicBindingParameters = array();

$types = '';
foreach ($mappedParameters as $mapping => $value) {
// replace first occurence only
$position = strpos($mappedQuery, $mapping);
if ($position !== false) {
$mappedQuery = substr_replace($mappedQuery, '?', $position, strlen($mapping));

Solution

After watching your class, some remarks I would like to do:

preparedQuery

In preparedQuery method, the declaration asks for two different ways of returning the data. I think it would be better just have one variable to manage all of them and check it with a switch:

Instead of:

public function preparedQuery($mappedQuery, $mappedParameters = array(), $returnInsertId = 0, $returnAffectedRows = 0)
{
    $dynamicBindingParameters = array();

    // [...]

    $return = $statement->get_result();
    $return = ($returnInsertId) ? (string)$this->insert_id : $return;
    $return = ($returnAffectedRows) ? $statement->affected_rows : $return;

    return $return;
}


This:

class simpleMysqli extends mysqli
{
    const RETURN_RESULT = 1;
    const RETURN_ID = 2;
    const RETURN_AFFECTED_ROWS = 3;

public function preparedQuery($mappedQuery, $mappedParameters = array(), $return = simpleMysqli::RETURN_RESULT)
{
    $dynamicBindingParameters = array();

    // [...]

    switch ($return) {                        
        case RETURN_RESULT:
        default:
            $statement->get_result();
            break;
        case RETURN_ID:
            $this->insert_id
            break;
        case RETURN_AFFECTED_ROWS:
            $statement->affected_rows
            break;
    }

    return $return;
}


This way is tidy, and easy to modify. You can use default to set an Exception if wished, but it's up to you depending on how strict you want to do if they don't provide the right string. Using constants helps you with having the name of the return in the code, what is better for readability.

Insert and Update

In your 'insert' and 'update' method, you use this piece of code to concatenate all the values in a string:

$mappedQueryColumnString = '(';
$mappedQueryValueString = 'VALUES(';

$mappedParameters = array();

$i = 0;
foreach ($data as $column => $value) {
    $mappedQueryColumnString .= ($i > 0) ? ", $column" : $column; 
    $mappedQueryValueString .= ($i > 0) ? ", :$column" : ":$column";
    $mappedParameters[":$column"] = ($value) ? $value : '';

    $i++;
}
$mappedQueryColumnString .= ')';
$mappedQueryValueString .= ')';


Well, that does the trick, but I think is better and easy using array and not mess with commas and creating that:

$mappedQueryColumns = array();
$mappedQueryValues = array();

foreach ($data as $column => $value) {
    $mappedQueryColumns[]  = $column; 
    $mappedQueryValues[]   = ":$column";
    $mappedParameters[":$column"] = ($value) ? $value : '';
}

// And here, you concatenate all without trouble:
$mappedQueryColumnString = '(' . implode(', ', $mappedQueryColumnValues) . ')';
$mappedQueryValueString =  'VALUES(' . implode(', ', $mappedQueryColumnValues) . ')';


That way you forget about comma, checks, etc...

get

I will give the user the option to select the fields their wishes, because a 'SELECT ' can be a bullet on the stomach to the performance if the table is big or with a lot of fields... I would allow an array of fields to be listed, and by default (better that than nothing). Something like:

public function get($checkColumn, $checkValue, $fields = '*')
{
    if (is_array($fields)) {
        $fieldString = implode(', ', $fields);                      
    } else {
        $fieldString = '*';
    }

    $mappedQuery = "SELECT $fieldString FROM $this->table WHERE $checkColumn = :$checkColumn LIMIT 1";


And lately, I think you should try to merge getSingle and getMulti, they pretty much do the same and the lines are repeated. Both could be using a private method with they feed with different params, deppending if they want a single or a multiple result.

Code Snippets

public function preparedQuery($mappedQuery, $mappedParameters = array(), $returnInsertId = 0, $returnAffectedRows = 0)
{
    $dynamicBindingParameters = array();

    // [...]

    $return = $statement->get_result();
    $return = ($returnInsertId) ? (string)$this->insert_id : $return;
    $return = ($returnAffectedRows) ? $statement->affected_rows : $return;

    return $return;
}
class simpleMysqli extends mysqli
{
    const RETURN_RESULT = 1;
    const RETURN_ID = 2;
    const RETURN_AFFECTED_ROWS = 3;


public function preparedQuery($mappedQuery, $mappedParameters = array(), $return = simpleMysqli::RETURN_RESULT)
{
    $dynamicBindingParameters = array();

    // [...]

    switch ($return) {                        
        case RETURN_RESULT:
        default:
            $statement->get_result();
            break;
        case RETURN_ID:
            $this->insert_id
            break;
        case RETURN_AFFECTED_ROWS:
            $statement->affected_rows
            break;
    }

    return $return;
}
$mappedQueryColumnString = '(';
$mappedQueryValueString = 'VALUES(';

$mappedParameters = array();

$i = 0;
foreach ($data as $column => $value) {
    $mappedQueryColumnString .= ($i > 0) ? ", $column" : $column; 
    $mappedQueryValueString .= ($i > 0) ? ", :$column" : ":$column";
    $mappedParameters[":$column"] = ($value) ? $value : '';

    $i++;
}
$mappedQueryColumnString .= ')';
$mappedQueryValueString .= ')';
$mappedQueryColumns = array();
$mappedQueryValues = array();

foreach ($data as $column => $value) {
    $mappedQueryColumns[]  = $column; 
    $mappedQueryValues[]   = ":$column";
    $mappedParameters[":$column"] = ($value) ? $value : '';
}

// And here, you concatenate all without trouble:
$mappedQueryColumnString = '(' . implode(', ', $mappedQueryColumnValues) . ')';
$mappedQueryValueString =  'VALUES(' . implode(', ', $mappedQueryColumnValues) . ')';
public function get($checkColumn, $checkValue, $fields = '*')
{
    if (is_array($fields)) {
        $fieldString = implode(', ', $fields);                      
    } else {
        $fieldString = '*';
    }

    $mappedQuery = "SELECT $fieldString FROM $this->table WHERE $checkColumn = :$checkColumn LIMIT 1";

Context

StackExchange Code Review Q#90997, answer score: 2

Revisions (0)

No revisions yet.