# Review/rate my new graph class

I have written a PHP class called "graph". It is a class that performs RESTful-like commands to a MySQL database. I have posted the GitHub repo.

Here is the code as well:

config.php

<?php
//standard database connection data

//these obviously need to be changed....

define("DBHOST","localhost");
define("MAIN_DB","main");

?>


database.class.php

<?php
require_once('config.php');  //set your database settings in this

class database{

public static function dbConnect($db){$link = mysql_connect(DBHOST, DBUSERNAME, DBPASSWORD);

if (!$link) {$response = array(
"object"        => "database",
"instrcut"      => "dbConnect",
"status"        => "fail",
"message"       =>  mysql_error()

);

}

if($db == ''){ //sets the$db var to the constant MAIN_DB if not otherwise set.
$db = MAIN_DB;$db_select = mysql_select_db($db); }else{$db_select = mysql_select_db($db); } if(!$db_select){
$response = array( "object" => "database", "instrcut" => "dbConnect", "status" => "fail", "message" => mysql_error() ); }else{$response = array(
"object"        => "database",
"instrcut"      => "dbConnect",
"status"        => "pass",
"message"       =>  "The database '$db' was connected to successfully." ); } return$response;

}

public static function conQuer($sql,$db){

$query = self::dbConnect($db);
$result = mysql_query($sql);

if($query&&$result){
$response = array( "object" => "database", "instrcut" => "conQuer", "status" => "pass", "message" => "The connection was established and the query was run successfully.", "data" =>$result
);
}else{
$response = array( "object" => "database", "instrcut" => "conQuer", "status" => "fail", "message" => "The conQuer method failed because there was a bad query." ); } return$response;
}

//end database class
}

?>


Finally, the big one:

graph.class.php

 <?php
//PHP-GRAPH class

require_once('database.class.php');

class graph{
//used to retrieve existing data

public static function get($data){$jsonData = json_encode($data); //decode json data$data = json_decode($jsonData); //////////////////////////////$db = $data->db;$table = $data->table;$field = $data->field;$value = $data->value;$request = array($field,$value);
//////////////////////////////

if($db&&$table&&$field&&$value){
////////////GET ALL THE AVAILABLE FIELDS FROM SELECTED TABLE////////////

//query to get the available fields
$result = database::conQuer("SHOW COLUMNS FROM$table",$db);$result = $result['data']; if (!$result) {

//return the failure response
$response = array("obj"=>"graph","instruct"=>"get","status"=>"fail","msg"=>mysql_error()); } if (mysql_num_rows($result) > 0) {
while ($row = mysql_fetch_assoc($result)) {

$field_array[] =$row['Field'];

}
}
/////////////////////////////////////////////////////////////////////////////

////////////GET ALL THE DATA FROM THE SELECTED FIELDS ////////////

$getQuery = database::conQuer("SELECT * FROM$table WHERE $field = '$value'",$db);$getQuery = $getQuery['data'];$queryCount = mysql_numrows($getQuery); while($row = mysql_fetch_array($getQuery)){ foreach($field_array as $fa){$value = $row["$fa"]; //get the value of the field
$field =$fa; //get the actual field name

//store in an array to be easily accessable
if($queryCount > 1){$getInfo[$field][] =$value;
}else{
$getInfo[$field] = $value; } } } //need to check if any data was actually recieved if(empty($getInfo)){
$response = array("obj"=>"$table","instruct"=>"get","status"=>"pass","msg"=>"Graph ran successfully, but no data was retrieved. Check the graph coordinates and try again.","data"=>"empty");
}else{
$response = array("obj"=>"$table","instruct"=>"get","request"=>$request,"data"=>$getInfo,"status"=>"pass","msg"=>"Data was successfully retrieved.");
}

}else{
$response = array("obj"=>"$table","instruct"=>"get","status"=>"fail","msg"=>"Graph couldn't get any data because all required parameters weren't passed.");
}

//if successful, this will return a JSON array, with the final value of "data".

return json_encode($response); } public static function post($data){
//used to create new data

//required parameters
$db =$data['db'];
$table =$data['table'];
$fieldData =$data['field'];

if($db&&$table){
if($fieldData){ //need to create the new record here$field_id = $table."_id"; //apart of the naming standards setup that the system must use to work...$createNewRecord = database::conQuer("INSERT INTO $db.$table ($field_id) VALUES (NULL);",$db);

if($createNewRecord){$newFieldId = mysql_insert_id(); //gets the lasat id of the query

foreach($fieldData as$value){
$field = array_search("$value", $fieldData);$insert = database::conQuer("UPDATE $db.$table SET $field = '$value' WHERE $table.$field_id =$newFieldId LIMIT 1 ;",$db);
}

$response = array("obj"=>"$table","instruct"=>"post","data"=>$fieldData,"status"=>"pass","msg"=>"Graph posted the data successfully."); }else{$response = array("obj"=>"$table","instruct"=>"post","status"=>"fail","msg"=>"Graph didn't run because there was a query error when creating a new value."); } }else{$response = array("obj"=>"$table","instruct"=>"post","status"=>"fail","msg"=>"Graph didn't run because no data values were passed."); } }else{$response = array("obj"=>"$table","instruct"=>"post","status"=>"fail","msg"=>"Graph didn't run because the required parameters were not passed."); } return json_encode($response);
}

public static function put($data){ //used to update existing data$db = $data['db'];$table = $data['table'];$field = $data['field'];$value = $data['value'];$ref = $data['ref'];$refval = $data['refval']; if($db&&$table&&$field&&$value&&$ref&&$refval){$put = database::conQuer("UPDATE $db.$table SET $field = '$value' WHERE $table.$ref = '$refval';",$db);
if($put){$putData = array($field,$value,$ref,$refval);

$response = array("obj"=>"$table","instruct"=>"put","data"=>$putData,"status"=>"pass","msg"=>"Graph updated the data successfully."); }else{$response = array("obj"=>"$table","instruct"=>"put","status"=>"fail","msg"=>"Graph didn't put any data in because the query failed."); } }else{$response = array("obj"=>"$table","instruct"=>"put","status"=>"fail","msg"=>"Graph didn't put any data in because the required parameters were not passed."); } return json_encode($response);

}

public static function delete($data){ //used to remove existing data //used to update existing$db = $data['db'];$table = $data['table'];$ref = $data['ref'];$refval = $data['refval']; if($db&&$table&&$ref&&$refval){$delete = database::conQuer("DELETE FROM $table WHERE $table.$ref = '$refval'",$db); if($delete){
$deleteData = array($ref,$refval);$response = array("obj"=>"$table","instruct"=>"delete","data"=>$deleteData,"status"=>"pass","msg"=>"Graph deleted the data successfully.");
}else{
$response = array("obj"=>"$table","instruct"=>"delete","status"=>"fail","msg"=>"Graph didn't delete any data in because the query failed.");
}
}else{
$response = array("obj"=>"$table","instruct"=>"delete","status"=>"fail","msg"=>"Graph didn't delete any data in because the required parameters were not passed.");
}

return json_encode($response); } //end graph class } ?>  I would really appreciate the community's view on the class. What do you guys think about it? What can I do better? Critiques are welcome. I have used the class in several sites that I've built and it really seemed to help me. I hope that maybe it could help someone else. Please post any comments, suggestions, etc. • Looks clean but not sure about mysql_*. Users on newer versions of PHP are going to get deprecated warnings everywhere, I'd switch to PDO. – elclanrs Oct 9 '13 at 1:45 • Please include the code you'd like to have reviewed. The link could die at any time, rendering the question useless. – Jamal Oct 9 '13 at 1:46 • Yes. That is a good idea. I will update that... – Daniel Sikes Oct 9 '13 at 1:46 ## 1 Answer A few points: 1. You should avoid mysql_query. It's deprecated. Use PDO or mysqli. • Read up on SQL injection attacks and use parametrized queries. 2. Your indentation seems to be all over the place (not sure if that's a result of copy-n-paste, haven't checked the github sources) • Use more spaces - they are cheap (e.g. }else{ should be at least } else {); 3. Comments should mainly describe reasons for non-obvious decision. Most of your comments seem to just describe what the code below is doing which you can see by looking at the code. Example: //decode json data$data = json_decode($jsonData);  Those comments add no value. Furthermore reiterating what the code does in the comments will usually lead to comments conflicting with code once a few rounds of refactoring have taken place. 4. Why is this called Graph? Graph means usually one of two things to me: Either a data structure with nodes and edges connecting the nodes or the visualization of some data. I can see neither of those here really. 5. You repeat a lot of string literals in your response creation. This should be refactored into a method. Makes refactoring later easier and avoids typos. Maybe even create a class encapsulating the response so you can do r = response::build(...); r.Add(key, value); r.toJson(); or something along those lines. • Most prominently you use instrcut in your database class but instruct in your graph class. I'd actually use command (instructions in computer terms are usually associated with more low level operations but YMMV) 6. Consider dealing with error cases first to reduce nesting. This makes the code easier to read when there are multiple levels. E.g.: if (!$db || !$table) { return response::build(...).toJson(); } if (!$fieldData)
{
return response::build(...).toJson();
}

// handle the main case

return response::build().toJson();


In regards to the response refactoring:

class response
{
private data = array();

public static function build($object,$command, $status,$message)
{
$res = new response();$res->data['object'] = $object; ... return$res;
}

public function toJson()
{
return json_encode(\$this->data);
}
}


Disclaimer: Above code might not be 100% correct but should get the idea across. It's been a long time since I worked with PHP.

• Thanks for the help Chris Wue! To answer your question about why I named it graph. Originally I visualized the setup like this : The database represented a x value, the table represented a y value, and the field represented a z value. In my mind, I was in a way, building a graph of a database. It could be called anything really... I just left it what I had originally called it. You are the 2nd person to mention the switch to PDO. I will be adding that, and then uploading it again. Also, I copied and pasted the code so thats why the indenting is crazy. It looked fine in my editor. – Daniel Sikes Oct 9 '13 at 2:43
• On point # 5. Could you please be more specific? What should a response look like? What do you mean it should be a method? Show me what you are talking about. I want to make this class better, for sure. So that others can use it. Do you like the idea of being able to perform rest commands to a database? Thanks, Dan – Daniel Sikes Oct 9 '13 at 3:12
• @DanielSikes: Fair enough. I updated my answer – ChrisWue Oct 9 '13 at 3:37