Thought I’d start a thread about some annoying/bad PHP programming code. The large PHP projects I get handed (which were started by cheap out-sourcing or self-taught programmers) tend to include these or similar over and over again.
Don't hardcode colours into php files (or even html files for that matter)
use css classes instead, so you only have to update a single css rule instead of every single php file in the whole app.
Original code:
if ($oddRow == 1) {
print '<'.'tr style="background-color: white;">';
$oddRow = 0;
}else{
print '<'.'tr style="background-color: #dddddd;">';
$oddRow = 1;
}
Replacement PHP code:
print '<'.'tr class=" . (($oddRow++%2) ? 'even' : 'odd') . ">';
New CSS code:
tr.odd{background:#FFF;} tr.even{background:#DDD;}
Feel free to post your own bad code below!
Limit the use of unnecessary HTML markup code:
reason: harder to get it working cross browser.
Original code: (this span was in EVERY td and th in the entire application)
print ''. $row['rcvd_dt'] .'
New Code:
print ''. $row['rcvd_dt'] .' ';
Accidental assignment within if statement:
This happens all to often, I just found one today in some code I'm fixing.
The error looks like this:
if($_REQUEST['action'] = 'delete'){ // run the delete code...
Of course, delete will run every time. This should be:
if($_REQUEST['action'] == 'delete'){ // run the delete code..
An even better practice to get into is swapping the variables around:
if('delete' == $_REQUEST['action']){ // run the delete code..
This way if you do accidentally use = instead of == you will get a PHP error.
Limit using 'print' or 'echo' to spit out large blocks of HTML code:
reason: when your IDE has some nice HTML features, it cannot apply these to php strings.
for example, in the below string there is a html error (an additional " in the view link) which is not picked up by the IDE until we take this html out of a php string:
Original PHP
print '
'. $row['sent_dt'] .'
'. $row['subject'] .'
'. $row['to'] .'
View
';
New PHP
?>
<?php echo $row['sent_dt'];?>
<?php echo $row['subject'];?>
<?php echo $row['to'];?>
View
try to use the correct HTML markup tags instead of span with a class
reason: cross browser nightmare ! and try not to name classes the same as HTML tags. confusing!
Original html code
Some Text New html code
Some Text (apply your css styles to "h2" instead of ".h2")
Passing / modifying superglobals incorrectly
My fav, I got into a fight over this one and caused some jobs to be lost
Passing super globals (like $_POST, $_GET, $_REQUEST) into a function, without adjusting the superglobal variable name. Similar code was in every function in the entire huge crappy app.
some_function($_POST);
function some_function($_POST){
$_POST['foo'] = 123;
}
This is wrong on so many levels. Like, urgh, re-declaring $_POST in the local function scope. Like. argghh. anywho.
Looking for an undefined index
I've seen the following statement a lot:
if($_GET['id']){
// Code
}
I think it often get's overlooked because PHP only outputs a notice, maybe some developers don't display all error reporting :notfunny:
You should be using:
if(isset($_GET['id'])){
// Code
}
PS: Nice posts so far, and good topic idea :)
Using PHP short tags
When reviewing Wordpress plugins here on codecanyon I see many developers using the PHP short tags like:
<?= $var ?>
instead of
<?php echo $var ?>
I know the second option is longer but that’s the only manner that will work on all PHP servers.
Firsh
July 21, 2010, 12:29pm
9
Great topic, it’s good to know that I’m familiar with most of these
Yup, good stuff in here. Have only just realized the evils of short tags, lots of editing in front of me!
A pet peeve of mine is when people continue to use the simple mysql_query for large projects. There are much better alternatives.
Using straight user data without sanitation. It’s just criminal to do.
Just use a simple clean function: http://dev-tips.com/featured/create-a-simple-input-sanitation-function-with-php
A pet peeve of mine is when people continue to use the simple mysql_query for large projects. There are much better alternatives. :)
fork();
Accidental assignment within if statement:
This happens all to often, I just found one today in some code I'm fixing.
The error looks like this:
if($_REQUEST['action'] = 'delete'){ // run the delete code...
Of course, delete will run every time. This should be:
if($_REQUEST['action'] == 'delete'){ // run the delete code..
An even better practice to get into is swapping the variables around:
if('delete' == $_REQUEST['action']){ // run the delete code..
This way if you do accidentally use = instead of == you will get a PHP error.
Destructive actions should not be mapped onto GET requests, so instead of using $_REQUEST, one should always stick to using $_POST.
