patternphpMinor
Update PHP function GetSQLValueString
Viewed 0 times
phpfunctionupdategetsqlvaluestring
Problem
The old function was:
I have changed in this way:
Is what I've made correct?
Can I remove
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
-
The
-
If you intended to return the original value when its not text, long, int etc. you should make it explicit with a
-
-
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.