I know there are duplicates of this question, and the answer was to refactor the code.
However I'm not sure where to begin with this particular function.
I'm practising my OOP API skills in PHP5 and decided to write my own little Database API.
I have a function with 4 nested if statements, Im not even sure if 4 nested if's are a lot.
But this piece of code just seems messy to me and was wondering if anybody could provide any tips of how to optimise, reduce if's etc.
How would this kind of function be written in a real-world scenario?
My Code Follows:
public function custom_query( $sql_query_string, $single_column = false){
$link = $this->_Link_ID;
// IF LINK IS VALID RESOURCE
if ( is_resource( $link ) ) {
$query_resource = mysql_query( $sql_query_string, $link );
// IF QUERY WAS VALID
if ( is_resource( $query_resource ) ) {
$this->_Query_ID = $query_resource;
$row_count = mysql_num_rows( $query_resource );
// IF $ROW_COUNT IS A NUMBER, VALID ROWS WERE FOUND
if ( is_numeric( $row_count ) ) {
if ( $single_column ){
$result_set = mysql_fetch_assoc( $query_resource );
return $result_set;
}
else {
$result_set = array();
for ( $row = 0; $row < $row_count; $row++ ) {
$result_set[$row] = mysql_fetch_assoc( $query_resource );
}
return ( object ) array ( 'row_count' => $row_count, 'result_set' => $result_set );
}
}
else {
die( "Failed To Retrieve Row Count Query: $sql_query_string MySQL Error: " . mysql_error( $link ) );
}
}
else {
die( "Invalid Query : $query_string. MySql : " . mysql_error( $link ) );
}
}
else {
die( "Query attempted without valid link resource. Query : $query_string" );
}
}
Thanks,
Alex
If you decide to stop using die() in your function you can rewrite your code to form:
class Yours {
protected $errno = 0;
protected $error = '';
public function custom_query( $sql_query_string, $single_column = false){
$link = $this->_Link_ID;
// IF LINK IS VALID RESOURCE
if ( !is_resource( $link ) ){
$this->error = "Query attempted without valid link resource. Query : $query_string";
$this->errno = -1;
return null;
}
...
}
}
Or you can use exceptions;
public function custom_query( $sql_query_string, $single_column = false){
$link = $this->_Link_ID;
// IF LINK IS VALID RESOURCE
if ( !is_resource( $link ) ){
throw new Exception( "Query attempted without valid link resource. Query : $query_string", -1);
}
...
}
I personally would go for exceptions... But I guess there are people that disagree with me and I'd like to read their arguments.
EDIT Exception example. We'll extend php Extension class:
class DbException {
protected $sql;
public __construct( $message = "", $code = 0, $sql = '', $previous = NULL){
parent::__construct( $message, $code, $previous);
$this->sql = $sql;
}
public function getSql(){
return $this->sql;
}
}
// In model:
throw new DbException( "Query attempted without valid link resource.", -1, $query);
// In main application:
try {
ob_start();
// run whole application
} catch( DbException &e){
ob_clean(); // Suppress all output so far
echo "<html><body><div class="error">" . htmlspecialchars( $e->getMessage()) . "</div>";
if( NOT_IN_PRODUCTION){
echo "<div class='sql'>" . htmlspecialchars( $e->getSql()) . "</div>";
}
echo "</body></html>";
}
Exceptions: