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

Performing database interactions

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

Problem

I wrote this CRUD class in PHP that has methods to do database interactions, but I want to know if it is good/secure/elegant?

It works fine and all and it hasn't ever given me any troubles, but I'm really trying to improve my coding habits, so any feedback would be great!

```
class DbCrud {

private $conn;

/**
* Connect to the database
* @param string $host
* @param string $user
* @param string $pass
* @param string $dbname
*/
function __construct($host='localhost',$user,$pass,$dbname) {
$this->conn = new mysqli($host, $user, $pass, $dbname);
}

/**
* Disconnect from database
*/
function __destruct() {
$this->conn->close();
}

/**
* Add items to database
* @param string $table
* @param array $items formatted as 'column'=>'value'
*/
function create($table,$items) {
$numcol = count($items);
$count = 0;
$query = "INSERT INTO $table (";
foreach($items as $key=>$value) {
$key = $this->conn->real_escape_string($key);
$count++;
$query .= "$key";
if($count $value) {
$key = $this->conn->real_escape_string($value);
$count++;
$query .= "'$value'";
if($count conn->query($query);
}

/**
* Read from the database
* @param string $table
* @param string $selection
* @param array $conditions formatted as WHERE column=>value
* @param array $order formatted as column=>direction
* @return array $data formatted as a multidementional array where first key is the row and the second key is the column name. Returns 0 if no data.
*/
function read($table,$selection="*",$conditions="",$order="") {
$numcol = count($conditions);
$count = 0;
$query = "SELECT $selection FROM $table";
if($conditions != "") {
$query .= " WHERE ";
foreach($conditions as $key=>$value) {

Solution

First:

function __construct($host='localhost',$user,$pass,$dbname) {


You shouldn't follow an optional argument by other mandatory params.

function __construct($host,$user,$pass,$dbname) {


or maybe

function __construct(array $config) {
    $defaults = array(
        'host'   => 'localhost',
        'user'   => 'root',
        'pass'   => 'root',
        'dbname' => 'default'
    );

    foreach($defaults as $key=>$value) {
        if(!isset($config[$key])) {
            $config[$key] = $value;
        }
    }

    $this->conn = new mysqli($config['host'], $config['user'], $config['pass'], $config['dbname']);
}


morover:

function create($table,$items) {


I think there is a possible naming conflicts. With "Create" I think to "Create a table". Maybe it's better rename this with "addRow" or "add".

/**
 * Read from the database
 * @param string $table
 * @param string $selection
 * @param array $conditions formatted as WHERE column=>value
 * @param array $order formatted as column=>direction
 * @return array $data formatted as a multidementional array where first key is the row and the second key is the column name. Returns 0 if no data.
 */
function read($table,$selection="*",$conditions="",$order="") {


see the $order param. It's an array as PHPDoc said, or it's a string? maybe you should write:

function read($table,$selection="*",$conditions="",$order=array()) {


and inside the function

if(!empty($order)) { // was if($order != "")


and for function/code itself, I think it's better a way like that (for example the create method but same procedure could be used anywhere):

$keys   = array_keys($items);
$values = array();

foreach($items as $val) {
    $values[] = $this->conn->real_escape_string($val);
}

$query = "INSERT INTO `$table` (`".implode("`,`",$keys."`) VALUES ('".implode("','",$values)."')";

Code Snippets

function __construct($host='localhost',$user,$pass,$dbname) {
function __construct($host,$user,$pass,$dbname) {
function __construct(array $config) {
    $defaults = array(
        'host'   => 'localhost',
        'user'   => 'root',
        'pass'   => 'root',
        'dbname' => 'default'
    );

    foreach($defaults as $key=>$value) {
        if(!isset($config[$key])) {
            $config[$key] = $value;
        }
    }

    $this->conn = new mysqli($config['host'], $config['user'], $config['pass'], $config['dbname']);
}
function create($table,$items) {
/**
 * Read from the database
 * @param string $table
 * @param string $selection
 * @param array $conditions formatted as WHERE column=>value
 * @param array $order formatted as column=>direction
 * @return array $data formatted as a multidementional array where first key is the row and the second key is the column name. Returns 0 if no data.
 */
function read($table,$selection="*",$conditions="",$order="") {

Context

StackExchange Code Review Q#79673, answer score: 3

Revisions (0)

No revisions yet.