Bad PHP programming practices

Every now and then I run into a bit of bad PHP code. Or just a bit of code which could be written better. I’ve included these below. Will add to it every time I find someone new:

---

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;}

Limit the use of unnecessary HTML markup code:

reason: harder to get it working cross browser. 
like below, you could style “td” tags just the same as a the “h2” span is.

Original code:

<td><span class="h2">'. $row['rcvd_dt'] .'</span></td>

New Code:

<td>'. $row['rcvd_dt'] .'</td>

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.
It’s a hard practice to get into, but well worth it in the long run.

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 '
	<td>
		'. $row['sent_dt'] .'
	</td>
	<td>
		'. $row['subject'] .'
	</td>
	<td>
		'. $row['to'] .'
	</td>
	<td align="center">
		<a href="composeemail.php?email_id='.$row['email_id'].'"">View</a>
	</td>';

New PHP

   ?>
	<td>
		<?php echo $row['sent_dt'];?>
	</td>
	<td>
		<?php echo $row['subject'];?>
	</td>
	<td>
		<?php echo $row['to'];?>
	</td>
	<td align="center">
		<a href="composeemail.php?email_id=<?php echo $row['email_id'];?>">View</a>
	</td>
   <?php

try to use the correct HTML markup tags instead of just using a class 

and try not to name classes the same as HTML tags. confusing!

Original html code

<span class="h2">Some Text</span>

New html code

<h2>Some Text</h2>

(apply your css styles to “h2” instead of “.h2”)

Passing / modifying superglobals incorrectly

Passing super globals (like $_POST, $_GET, $_REQUEST) into a function, without adjusting the superglobal variable name:

some_function($_POST);
function some_function($_POST){
    $_POST['foo'] = 123;
} 

This is wrong on so many levels.