PHP Developers Network

A community of PHP developers offering assistance, advice, discussion, and friendship.
 
Loading
It is currently Sat Dec 07, 2019 1:29 am

All times are UTC - 5 hours




Post new topic Reply to topic  [ 7 posts ] 
Author Message
PostPosted: Wed Nov 23, 2011 3:41 am 
Offline
Forum Commoner

Joined: Mon Nov 21, 2011 1:16 am
Posts: 69
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.

Syntax: [ Download ] [ Hide ]
<?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);

}



?>


Top
 Profile  
 
PostPosted: Wed Nov 23, 2011 5:36 am 
Offline
Forum Regular
User avatar

Joined: Tue Sep 28, 2010 11:41 am
Posts: 984
Location: Columbus, Ohio
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:
Syntax: [ Download ] [ Hide ]
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:
Syntax: [ Download ] [ Hide ]
$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 :
Syntax: [ Download ] [ Hide ]
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.


Top
 Profile  
 
PostPosted: Wed Nov 23, 2011 7:17 am 
Offline
DevNet Master
User avatar

Joined: Sun Feb 15, 2009 12:08 pm
Posts: 2794
Location: .za
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


Top
 Profile  
 
PostPosted: Wed Nov 23, 2011 3:45 pm 
Offline
Forum Commoner

Joined: Mon Nov 21, 2011 1:16 am
Posts: 69


Top
 Profile  
 
PostPosted: Wed Nov 23, 2011 4:48 pm 
Offline
DevNet Master
User avatar

Joined: Sun Feb 15, 2009 12:08 pm
Posts: 2794
Location: .za

_________________
“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


Top
 Profile  
 
PostPosted: Thu Nov 24, 2011 2:54 am 
Offline
Forum Regular
User avatar

Joined: Tue Sep 28, 2010 11:41 am
Posts: 984
Location: Columbus, Ohio


Top
 Profile  
 
PostPosted: Thu Nov 24, 2011 11:47 pm 
Offline
Forum Contributor

Joined: Sat Jan 03, 2009 4:27 pm
Posts: 148


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 7 posts ] 

All times are UTC - 5 hours


Who is online

Users browsing this forum: No registered users and 4 guests


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Jump to:  
Powered by phpBB® Forum Software © phpBB Group