Pls help and advice

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
laics
Forum Newbie
Posts: 7
Joined: Mon Apr 07, 2008 2:21 pm

Pls help and advice

Post by laics »

Dear all,

Below are my code for my class Member. It has been running really slow in my site. I need some comments and suggestion on how to speed up the execution of this code. Thanks.

Code: Select all

 
class Member
{
    function getMemberData($uid){
        $query="SELECT * FROM users WHERE U_ID LIKE '".$uid."'";
        $search=mysql_query($query);
        $member = mysql_fetch_array( $search );
        $list->firstname=str_replace('"','"',($member['FIRSTNAME']));
        $list->lastname= str_replace('"','"',($member['NAME']));
        $list->gender=$member['GENDER'];
        $list->photo=getUsrProfImg($uid);
        $list->opt_personal_display=$member['OPT_PERSONAL_DISPLAY'];
        $list->crt_tms=$member['CRT_TMS'];
        $list->login_id=$member['LOGIN'];
        $list->email=$member['EMAIL'];
        $townInfo=getTownState($member['TOWN_ID']);
        $list->town=$townInfo['TOWN'];
        $list->state_id=$townInfo['STATE_ID'];
        $list->state=$townInfo['STATE'];
        $list->postcode=$townInfo['POSTCODE'];
        $list->uInfo['sshort']=$townInfo['STATE_SHORT'];
        $list->type=$member['TYPE'];
        $list->userId=$member['U_ID'];
        $list->lastlog=$member['LAST_LOGIN'];
        $list->ques=$member['QUES'];
        $list->ans=$member['ANS'];
        $dob=trim($member['DOB']);
        $list->dt=substr($dob, 0,2);
        $list->mn=substr($dob, 2,2);
        $list->yr=substr($dob, 4, 4);
        mysql_free_result($search);
        return $list;
    }
}
 
The class is being called as below:

$m = Member::getMemberData($member_id);

echo $m->firstname.", ".$m->lastname;
anto91
Forum Commoner
Posts: 58
Joined: Mon Mar 10, 2008 10:59 am
Location: Sweden

Re: Pls help and advice

Post by anto91 »

I think this is probaly the speed thief

Code: Select all

 
SELECT * FROM users WHERE U_ID LIKE $UID
 
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Re: Pls help and advice

Post by Christopher »

Yes, is that column indexed? And why is this a class? Is should just be a function.
(#10850)
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Re: Pls help and advice

Post by Christopher »

laics wrote:Yes the columns are indexed. I do it as class thought it is easier to maintain. May I know whether you have better suggestion on how should modify the class? o should i separate them into different methods respectively? Thanks.
The class has no properties, only one function, and you are calling it statically. There is no difference between what you are doing and a plain function except a little overhead for the static class call.
(#10850)
laics
Forum Newbie
Posts: 7
Joined: Mon Apr 07, 2008 2:21 pm

Re: Pls help and advice

Post by laics »

The class has no properties, only one function, and you are calling it statically. There is no difference between what you are doing and a plain function except a little overhead for the static class call.
Is it means that i can solved this issue by separating all the method like getfirstname(), getsecondname, getuserid and so on?
User avatar
John Cartwright
Site Admin
Posts: 11470
Joined: Tue Dec 23, 2003 2:10 am
Location: Toronto
Contact:

Re: Pls help and advice

Post by John Cartwright »

Like others have mentioned -- the slowness lies in the query. Why are you using LIKE? Why not a straight equals?
laics wrote:Yes the columns are indexed.
You said columns (plural). Most often the only field that should be an index is the primary key. Having too many indexes is just as harmful as none at all.

So which fields are indexed?
laics
Forum Newbie
Posts: 7
Joined: Mon Apr 07, 2008 2:21 pm

Re: Pls help and advice

Post by laics »

You said columns (plural). Most often the only field that should be an index is the primary key. Having too many indexes is just as harmful as none at all.
 
So which fields are indexed?
 
Is the equal execute faster than the LIKE? Can you explain furthermore on that. It is new to me.
Yes.. I have indexed the name column too. is it fine?
Thanks.
User avatar
Chris Corbyn
Breakbeat Nuttzer
Posts: 13098
Joined: Wed Mar 24, 2004 7:57 am
Location: Melbourne, Australia

Re: Pls help and advice

Post by Chris Corbyn »

laics wrote:Is the equal execute faster than the LIKE? Can you explain furthermore on that. It is new to me.
Yes.. I have indexed the name column too. is it fine?
Thanks.
LOTS faster.

Code: Select all

$query="SELECT * FROM users WHERE U_ID = '".$uid."'";
Also, if $uid comes from a form or a GET variable you're open to attack with this code.
laics
Forum Newbie
Posts: 7
Joined: Mon Apr 07, 2008 2:21 pm

Re: Pls help and advice

Post by laics »

Hi Chris Corbyn,

Thanks for your reply. $uid is come from $_GET too. May I know how to solve this problem to secure it from open attack? $uid is retrieved after an encryption from $_GET. Thank you.
User avatar
Chris Corbyn
Breakbeat Nuttzer
Posts: 13098
Joined: Wed Mar 24, 2004 7:57 am
Location: Melbourne, Australia

Re: Pls help and advice

Post by Chris Corbyn »

laics wrote:Hi Chris Corbyn,

Thanks for your reply. $uid is come from $_GET too. May I know how to solve this problem to secure it from open attack? $uid is retrieved after an encryption from $_GET. Thank you.

Code: Select all

$query="SELECT * FROM users WHERE U_ID = '".mysql_real_escape_string($uid)."'";
Post Reply