Pulling a single MySQL value

Coding Critique is the place to post source code for peer review by other members of DevNetwork. Any kind of code can be posted. Code posted does not have to be limited to PHP. All members are invited to contribute constructive criticism with the goal of improving the code. Posted code should include some background information about it and what areas you specifically would like help with.

Popular code excerpts may be moved to "Code Snippets" by the moderators.

Moderator: General Moderators

Post Reply
Hermit TL
Forum Commoner
Posts: 69
Joined: Mon Nov 21, 2011 12:16 am

Pulling a single MySQL value

Post by Hermit TL »

Quite often I need to pull just one specific value from a MySQL database so I wrote this function to do just that. I've been using it for a while and haven't had any problems with it at all. I was just wondering if there is anything wrong with it. Thanks to anyone who takes the time to examine my function and provide comments.

Code: Select all

<?php

include_once("sql_connect.php");



//Hermit TLs' my_get function v0.1

//This function pulls a single value from your 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){

session_start();

if (!$where){

	$where = $_SESSION['id'];

}

	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>

	');

	$results = mysql_fetch_array($getdata);

	$got = $results[$value];

		return ($got);

	unset($value, $table, $where, $error, $override);

}



?>
User avatar
twinedev
Forum Regular
Posts: 984
Joined: Tue Sep 28, 2010 11:41 am
Location: Columbus, Ohio

Re: Pulling a single MySQL value

Post by twinedev »

Calling session_start() from within a function that will be most likely called many times is not good, as it will produce notices (you may have them not displaying or logging, but IMO best for a script to never produce anything and a development environment should display ALL levels of error reporting, including Notices.

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)

A better place to have this would be on something that gets called early on only once, or at the very least should do something like:

Code: Select all

if (!session_id) { session_start(); }
Make sure you do proper variable cleansing before passing them to this function (ie. using mysql_real_escape_string(). ANd if Field/Table can possibly be affected by user input, Change your query string to:

Code: Select all

$query = "SELECT `" . str_replace('`','',$value) . "` FROM `" . str_replace('`','',$table) . "` WHERE " . $where
You have not code in place for what if there were no rows returned.

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;
}
The unset at the end, 1, no necessary as once the function is exited, unless they were declared static, won't exist anyhow, and 2. nothing in a function gets executed after a return statement is called.
User avatar
social_experiment
DevNet Master
Posts: 2793
Joined: Sun Feb 15, 2009 11:08 am
Location: .za

Re: Pulling a single MySQL value

Post by social_experiment »

I would throw an exception instead of using die() to check for any failing on the script.

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 which might result from your mysql functions.

There's also no checking of what type of data you receive so this is an area where you can improve (twinedev already touched on the issue). Another thing which might make no difference is to use FALSE instead of NULL but this is just a personal preference.
“Don’t worry if it doesn’t work right. If everything did, you’d be out of a job.” - Mosher’s Law of Software Engineering
Hermit TL
Forum Commoner
Posts: 69
Joined: Mon Nov 21, 2011 12:16 am

Re: Pulling a single MySQL value

Post by Hermit TL »

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;
	}

}



?>
User avatar
social_experiment
DevNet Master
Posts: 2793
Joined: Sun Feb 15, 2009 11:08 am
Location: .za

Re: Pulling a single MySQL value

Post by social_experiment »

Hermit TL wrote: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.
I see

Code: Select all

<?php
 $getdata = mysql_query($query);
 if (!$getdata) {
     throw new Exception('Error message');
 }
 else {
     // continue processing
 }
//
# when using the function you would do something like this
try {
    my_get($value, $table, $where, $error = NULL, $override = NULL);
}
catch (Exception $e) {
    // echoes "Error Message"
    echo $e->getMessage();
}
 ?>
The Manual on Exceptions
http://php.net/manual/en/language.exceptions.php
Hermit TL wrote:Where do you put the @ sign to suppress warning messages?
Before the mysql_query() function i.e @mysql_query($yourQuery); This means the warning message resulting from a failed query will not be echoed to the browser but lets you handle it yourself; like with an exception. More useful when the script is live, during production leave it so you can find errors / problems easier.
“Don’t worry if it doesn’t work right. If everything did, you’d be out of a job.” - Mosher’s Law of Software Engineering
User avatar
twinedev
Forum Regular
Posts: 984
Joined: Tue Sep 28, 2010 11:41 am
Location: Columbus, Ohio

Re: Pulling a single MySQL value

Post by twinedev »

Hermit TL wrote: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?

Yes you are correct on the fix, I must have been tired when I wrote that.... Sorry, but glad you figured out what I meant.

-Greg
MichaelR
Forum Contributor
Posts: 148
Joined: Sat Jan 03, 2009 3:27 pm

Re: Pulling a single MySQL value

Post by MichaelR »

Hermit TL wrote: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.
I didn't think booleans could be returned from (or stored in) a database. Aren't the data types used one of string, integer, double, blob, or NULL?
Post Reply