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 09-30-2003, 03:21 AM   #1
tuf-web.co.uk
what does this func do?
 
tuf-web.co.uk's Avatar
 
Join Date: Jan 2003
Location: TUF Ville
Posts: 582
my admin page code

heres my admin page which i have updated quite a few times

i know i should add some comments in, but i was only a beginner when i started this and i havent the time to run through the whole code again to comment it (though i will later)

anyways here it is, say your worst about it

but dont be mean

clicky here

comms
__________________
<?php echo $signature; ?>
-----------------------------------------
visit www.theutherfish.com for free web logs (blogs)
tuf-web.co.uk is offline   Reply With Quote
Old 09-30-2003, 01:33 PM   #2
LordShryku
kung foo code monkey
 
LordShryku's Avatar
 
Join Date: Aug 2002
Location: Occupational Hypnotherapy
Posts: 7,473
Well, more personal opinions than "coding standards", but my two cents:

1) The multiple echo statements in you functions could be turned into one echo statement. This would help a bit with dalecosp's need to get his HTML source formatted right. And it's just a bit easier/prettier.

2) The long if/elseif statements should be made into switch statements. Prime example...
PHP Code:
if ($_GET[mode] == 'news') {
    
$admin = "New News";
}
else if (
$_GET[mode] == 'edit') {
    
$admin = "Add/Change Edit";
}
else if (
$_GET[mode] == 'joke') {
    
$admin = "New Joke";
}
else if (
$_GET[mode] == 'cheats') {
    
$admin = "New Cheat";
}
else if (
$_GET[mode] == 'poll') {
    
$admin = "New Poll";
}
else if (
$_GET[mode] == 'users') {
    
$admin = "Users Admin";
}
else if (
$_GET[mode] == 'version') {
    
$admin = "Version History";
}
else if (
$_GET[mode] == 'counter') {
    
$admin = "Counter";
}
else if (
$_GET[mode] == 'enews') {
    
$admin = "Edit News";
}
else if (
$_GET[mode] == 'guest') {
    
$admin = "Guestbook";
}
else if (
$_GET[mode] == 'blog') {
    
$admin = "Blogs";
}

// made into a switch statement...

switch($_GET[mode]) {
    case
"news":
        
$admin = "New News";
        break;

    case
"edit":
        
$admin = "Add/Change Edit";
        break;

    case
"joke":
        
$admin = "New Joke";
        break;

    case
"cheats":
        
$admin = "New Cheat";
        break;

    case
"poll":
        
$admin = "New Poll";
        break;

    case
"users":
       
$admin = "Users Admin";
        break;

    case
"version":
        
$admin = "Version History";
        break;

    case
"counter":
        
$admin = "Counter";
        break;

    case
"enews":
        
$admin = "Edit News";
        break;

    case
"guest":
        
$admin = "Guestbook";
        break;

    case
"blog":
        
$admin = "Blogs";
        break;

    default:
        break;
}
planetsim did a good article on this that was made sticky on one of the forums. Can;t remember which one, but I'll link it once I find it.

Didn't get through it all, but everything else I saw looks pretty nice. GJ
LordShryku is offline   Reply With Quote
Old 09-30-2003, 01:49 PM   #3
jstarkey
Civilian
 
jstarkey's Avatar
 
Join Date: Jul 2002
Location: 9500 ft
Posts: 1,104
You also have some array keys that aren't quoted. It's generally a good idea to do so in order to avoid stepping on any keys names that may become reserved in the future.
jstarkey is offline   Reply With Quote
Old 09-30-2003, 03:02 PM   #4
LordShryku
kung foo code monkey
 
LordShryku's Avatar
 
Join Date: Aug 2002
Location: Occupational Hypnotherapy
Posts: 7,473
Here's that link....

http://www.phpbuilder.com/board/show...5&pagenumber=2
LordShryku is offline   Reply With Quote
Old 09-30-2003, 03:06 PM   #5
Sxooter
Chamberlain
 
Sxooter's Avatar
 
Join Date: Aug 2002
Location: Denver, CO
Posts: 4,056
Quote:
Originally posted by jstarkey
You also have some array keys that aren't quoted. It's generally a good idea to do so in order to avoid stepping on any keys names that may become reserved in the future.
Note that as of PHP 4.3.something the use of unquoted identifiers in arrays will result in a warning during run time.
__________________
PostgreSQL version 8.4 is now in beta! New features in the works: Updateable Views, WITH queries including recursive, On-disk bitmap indexes, and improved partitioning. woot!
Sxooter is offline   Reply With Quote
Old 10-01-2003, 02:29 AM   #6
Moonglobe
Better fan than rebelo!
 
Moonglobe's Avatar
 
Join Date: Apr 2003
Location: brain://localhost:left-side
Posts: 2,381
actually as of a long time ago, on any setup running error_reporting(E_ALL)
__________________
there's no place i can be, since i found serenity.
Moonglobe is offline   Reply With Quote
Old 10-01-2003, 08:36 AM   #7
The Chancer
Unrated...
 
The Chancer's Avatar
 
Join Date: Aug 2002
Location: Flying... always flying (apart from when crashed)
Posts: 1,299
Doesn't it just give a notice rather than a warning, and let you know that is assumed it to be $info['var' ] ? (Just being picky...)
__________________
The Chancer
----------------
Does This Mean I'm Bored
The Chancer is offline   Reply With Quote
Old 10-01-2003, 10:45 AM   #8
Sxooter
Chamberlain
 
Sxooter's Avatar
 
Join Date: Aug 2002
Location: Denver, CO
Posts: 4,056
Quote:
Originally posted by The Chancer
Doesn't it just give a notice rather than a warning, and let you know that is assumed it to be $info['var' ] ? (Just being picky...)
Yep. Sorry, hadn't checked the actual text in a while.
__________________
PostgreSQL version 8.4 is now in beta! New features in the works: Updateable Views, WITH queries including recursive, On-disk bitmap indexes, and improved partitioning. woot!
Sxooter is offline   Reply With Quote
Old 10-01-2003, 10:47 AM   #9
Moonglobe
Better fan than rebelo!
 
Moonglobe's Avatar
 
Join Date: Apr 2003
Location: brain://localhost:left-side
Posts: 2,381
Quote:
Originally posted by The Chancer
Doesn't it just give a notice rather than a warning, and let you know that is assumed it to be $info['var' ] ? (Just being picky...)
yup it suuuuure does.
__________________
there's no place i can be, since i found serenity.
Moonglobe is offline   Reply With Quote
Old 10-02-2003, 05:57 AM   #10
The Chancer
Unrated...
 
The Chancer's Avatar
 
Join Date: Aug 2002
Location: Flying... always flying (apart from when crashed)
Posts: 1,299
Oh well - was my own fault for updating my PHP and finding that loads of my old code fell into that trap...

After seeing it for the n000'th time, I had to say something...
__________________
The Chancer
----------------
Does This Mean I'm Bored
The Chancer 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 06:29 AM.






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.