The chunk to return the value could be changed to this :
Code: Select all
if (mysql_num_rows($getdata)==0) {
return FALSE;
}
else {
$return_val = mysql_result($getdata,$value);
mysql_free_result($getdata);
return FALSE;
}
Wouldn't that return FALSE every time? Did you mean:
Code: Select all
if (mysql_num_rows($getdata)==0) {
return FALSE;
}
else {
$return_val = mysql_result($getdata,$value);
mysql_free_result($getdata);
return $return_val;
}
or am I missing something?
Make sure you do proper variable cleansing before passing them to this function
Thanks; this is not done because all values are supplied by hard code (none from the user) when using this function.
IMO best for a script to never produce anything and a development environment should display ALL levels of error reporting, including Notices
You're right, I did have problems with this in the past when moving from my dev machine to a live test environment. (I'll look into fixing that.)
Also you will never be able to use this function once output is started without generating warnings. (But if you program like me, by the time you start outputting anything, all the info has been collected)
Yup. I never ever send output before all the input is collected. (That's good to know though.)
Another thing which might make no difference is to use FALSE instead of NULL but this is just a personal preference.
The reason I prefer NULL over FALSE is that sometime a result of FALSE would be correct if I am trying to pull a TRUE/FALSE value from the database.
There's also no checking of what type of data you receive so this is an area where you can improve
I wanted to use this function for any data types, so instead of having the function check that, I have code for that outside the function checking what it returned.
I would throw an exception instead of using die() to check for any failing on the script.
How would I do that?
You don't say explicitly if you use it for production purposes but if you do you might want to use the @ sign to suppress potential warning messages
It will eventually be used for production, but not for a while. Where do you put the @ sign to suppress warning messages? (I'm not going to add it until I think there are no warnings and have tested it on a production machine.)
The slightly (
completely untested) modified function:
Code: Select all
<?php
//*********************************
//*********************************
//*** my_get.php by Hermit TL v0.2a
//***
//*** pulls a single existing value
//*** from a MySQL database
//***
//*** Required Values
//*** $value : Column name of requested value
//*** $table : Table name of where column resides
//*** $where : ie. username=someuser
//***
//*** Optional Values
//*** $error : Custom error to be displayed
//*** $override : Use this MYSQL statment to query (if this value is specified all other values will be ignored)
//*********************************
//*********************************
function my_get($value, $table, $where, $error = NULL, $override = NULL){
if(!$override){
$query = "SELECT " . $value . " FROM " . $table . " WHERE " . $where;
}else{
$query = $override;
}
$getdata = mysql_query($query) or die ('
<table class="page">
<tr><td class="error">Error returned from my_get function.</td></tr>
<tr><td class="error">
Error: '.mysql_error() .
'</td></tr>
<tr><td class="error">'.
$query . '
</td></tr>
<tr><td class="error">
'.$error.'
</td></tr>
</table>
');
if (mysql_num_rows($getdata)==0) {
return FALSE;
}
else {
$return_val = mysql_result($getdata,$value);
mysql_free_result($getdata);
return FALSE;
}
}
?>