To register for an Internet.com membership to receive newsletters and white papers, use the Register button ABOVE.
To participate in the message forums BELOW, click here
PHPBuilder.com  
 

 

Go Back   PHPBuilder.com > PHP Help > Code Critique

Code Critique Having someone critique your code is always a great way to hone the skills. Stop in and post your code to see what your peers may have done differently.

Reply
 
Thread Tools Rate Thread Display Modes
Old 12-16-2003, 03:44 PM   #1
benson
Oant Sjen!!
 
Join Date: May 2003
Location: Tjalleberd,Friesland,Netherlands
Posts: 233
Functions

So give me critique

PHP Code:
<?php
include "http://www.streetfreaks.nl/includes/db.php";
#----------------------------Verklaring-------------------------------------------#
#vb. nick("user_level=3","<br>");                          #
#vb. nicklogin("20","<br>"); 20 staat voor het aantal echo's                  #
#vb. verjaardag(); stuurt altijd mail naar een persoon die jarig is op dat moment.#
#---------------------------------------------------------------------------------#



function nick($a,$b)
{
$sql = mysql_query("SELECT nicknaam FROM users WHERE ".$a);
while (
$row = mysql_fetch_object($sql))
    {
echo
$row->nicknaam.$b;
    }
mysql_free_result();
mysql_close();
}
function
nicklogin($a,$b)
{
$sql = mysql_query("SELECT nicknaam FROM users SORT BY last_visit LIMIT $a");
while (
$row = mysql_fetch_object($sql))
    {
        echo
$row->nicknaam.$b;
    }
mysql_free_result();
mysql_close();
}
function
verjaardag()
{
$sql = mysql_query("SELECT email_adres FROM users WHERE geb_datum=date(j-n-Y)");
$check = mysql_num_rows($sql);  
if (
$check > 0)
{
while (
$row = mysql_fetch_object($sql))
    {
$to = $row->email_adres;
$sub = "Gefeliciteerd";
$headers  = "MIME-Version: 1.0\r\n";
$headers .= "Content-type: text/html; charset=iso-8859-1\r\n";
$headers .= "From: Streetfreaks.nl <streetfreaks@streetfreaks.nl>\r\n";
$message = "
    Gefeliciteerd !!
    <img src=\"http://www.streetfreaks.nl/images/verjaardag.jpg\">
    "
;
mail($mailto, $subject, $message, $headers);
    }
}
mysql_free_result();
mysql_close();
}
?>
benson is offline   Reply With Quote
Old 12-16-2003, 04:11 PM   #2
LordShryku
kung foo code monkey
 
LordShryku's Avatar
 
Join Date: Aug 2002
Location: Occupational Hypnotherapy
Posts: 7,473
Well, personally when I'm making functions, or just using any variables, there's a meaning behind the naming of the variable. Sure, you may have to type a couple extra letters(oh my!), but at least I know what each one is. The whole $a and $b thing just doesn't work. Also, you may want to show an example of how these will work. SELECT nicknaam FROM users WHERE ".$a means you're not just passing a name to the function, but also a fieldname, and some formatting.
LordShryku is offline   Reply With Quote
Old 12-17-2003, 09:00 AM   #3
Shrike
Not Yet Involved
 
Shrike's Avatar
 
Join Date: Oct 2003
Location: The Eighth, Sursamen
Posts: 2,254
PHP Code:
$sql = mysql_query("SELECT email_adres FROM users WHERE geb_datum=date(j-n-Y)");
will pass 'date(y-n-Y)' to MySQL as a literal string?
__________________
The Hundredth Idiot
Shrike is offline   Reply With Quote
Old 12-17-2003, 09:02 AM   #4
Tristan Wells
Member
 
Join Date: Oct 2003
Location: Louisiana
Posts: 38
mysql_fetch_object will slow you script eventually.
__________________
The girls at school call me nerdalicious.
Tristan Wells is offline   Reply With Quote
Old 12-17-2003, 05:19 PM   #5
planetsim
code | beer > sleep
 
Join Date: Sep 2002
Location: aus
Posts: 4,826
Not to mention when using OOP you can confuse yourself and reuse object variables. Try to use mysql_fetch_array()
__________________
Dont be lazy Search
And use the Manual
Webmobo - Open Source News Script | Portfolio / Blog | Against TCPA
planetsim is offline   Reply With Quote
Old 01-05-2004, 12:13 AM   #6
superwormy
Senior Member
 
Join Date: Aug 2002
Location: Connecticut
Posts: 1,556
No database abstraction layer?

No checking of vars for malicios input before you use them in queries?

Calling mysql_close() on a connection that, from what I've seen, hasn't even been opened?

Ugly indents ( but that might just be the forum )?

I personally hate functions that just echo() data too, its nicer if they return; it so that I can manipulate it later and its a little more reusable, but thats just a matter of prefernece.
__________________
- Keith ( AIM: superwormy )

www.uglyslug.com || www.worminator.com || www.academickeys.com || www.floridajukido.com
superwormy is offline   Reply With Quote
Reply

Bookmarks


Currently Active Users Viewing This Thread: 1 (0 members and 1 guests)
 
Thread Tools
Display Modes Rate This Thread
Rate This Thread:

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is Off
HTML code is Off
Forum Jump


All times are GMT -4. The time now is 03:21 PM.






Acceptable Use Policy

internet.comMediabistrojusttechjobs.comGraphics.com

WebMediaBrands Corporate Info


Advertise | Newsletters | Feedback | Submit News

Legal Notices | Licensing | Permissions | Privacy Policy


Powered by vBulletin® Version 3.7.2
Copyright ©2000 - 2009, Jelsoft Enterprises Ltd.