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

Update PHP function GetSQLValueString

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

Problem

The old function was:



I have changed in this way:

real_escape_string($theValue); 
    switch ($theType) {
        case "text":
          $theValue = ($theValue != "") ? "'" . $theValue . "'" : "NULL";
          break;
        case "long":
        case "int":
          $theValue = ($theValue != "") ? intval($theValue) : "NULL";
          break;
        case "double":
          $theValue = ($theValue != "") ? "'" . doubleval($theValue) . "'" : "NULL";
          break;
        case "date":
          $theValue = ($theValue != "") ? "'" . $theValue . "'" : "NULL";
          break;
        case "defined":
          $theValue = ($theValue != "") ? $theDefinedValue : $theNotDefinedValue;
          break;
    }
    return $theValue;
}
?>


Is what I've made correct?

Can I remove get_magic_quites_gpc() if I use PHP version 5.3.2?

Solution

PHP is not my area of expretise, so just some generic notes:

-
The function create a new MySQL connection on every call which could be a bottleneck in your application. You could use persistent connections with a p: prefix but others usually suggest PDO as a better alternative of database handling.

-
The $the prefix on most of the variables seem redundant (it doesn't add too much to them), I'd avoid it.

-
If you intended to return the original value when its not text, long, int etc. you should make it explicit with a default case inside the switch statement. If not, throw an exception in the default case and sign the error. (The Pragmatic Programmer: From Journeyman to Master by Andrew Hunt and David Thomas: Dead Programs Tell No Lies.)

-
case "text" and case "date" contains the same logic, you could use fall through there too.

Context

StackExchange Code Review Q#42497, answer score: 2

Revisions (0)

No revisions yet.