patternphpMinor
Performing database interactions
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) {
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:
You shouldn't follow an optional argument by other mandatory params.
or maybe
morover:
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".
see the
and inside the function
and for function/code itself, I think it's better a way like that (for example the
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.