17 PHP Practices That Should Be Banished Forever

The following is a list of 17 PHP programming practices that in my opinion should be banished into oblivion. Forever and ever. Amen. Please note that this list is in no particular order. So, without further ado:



1. Using Register Globals

Most PHP programmers will have heard of this at least once. ‘Don’t rely on register globals!’. This issue has been around for quite some time. The disturbing thing is that still, in the year 2009, I find that many programmers still rely on this now deprecated feature of PHP. Using register globals is a bad idea. Stop relying on it. As a matter of fact turn it in off your php.ini configuration file. As of PHP 4.2.0 (if you’re using this or an older version you really should consider upgrading to a newer version, come on its free!) the default setting went from on to off. You absolutely do NOT need rogue variables swimming around in your application.

2. Relying on Magic Quotes

One of the inherent downsides to PHP is its ease of use. In attempting to make things as easy as possible for developers, the creators of the language have, with good intentions, put in place some features that are in direct conflict with good programming practice. When enabled, magic quotes ‘automagically’ escapes all user input, that is, renders it safe to be placed in a database. This creates a potential security hole in your application because you need to always know if its on or off. If its off and you don’t manually escape the user input you leave yourself susceptible to SQL injection attacks. If its on and you do manually escape user input then you will end up with double escaped data filled with lots of annoying backslashes. The best thing to do is to turn it off if you can and escape and unescape data manually using the addslashes() and stripslashes() functions respectively.

3. Not Sanitizing and Validating User Input

This is not limited to PHP, it is a general programming mantra independent of chosen language or platform; never ever trust user input. It is alarming, the number of unseasoned developers who accept user input via an html form or request data and simply throw this data right into their application via variables or a database without employing any form of input validation or sanitization. This is extremely dangerous and in many cases comes right back to laziness. Barring those who really just don’t know any better, there are some developers out there that are aware of all the risks but are simply just too lazy to write an extra few lines of code to ensure that the data submitted by users is what it should be. It is certainly not enough to rely on client side scripting as client side scripts can easily be circumvented. Take the time to properly sanitize and validate all user input. Its more than worth it.

4. Mixing HTML, PHP, SQL, JS, CSS, Etc, Etc

This just looks ugly and makes for a giant pot of spaghetti. Avoid doing it at all costs. If you must (though it defeats me as to why anyone ever ‘must’), at least separate each language into a separate section of the file as best as possible.

5. Using Short/ASP Style Tags

This is bad for a zillion reasons. I’ll give you two, one for each style. Short tags (``) cause conflicts with xml parsers because the xml language uses these tags as well. The concept of ASP style tags (`<% //code %>`) is an utterly stupid one simply because PHP is not ASP. Don’t use them. No excuses, just don’t use them.

6. Not Documenting Code

Any code that took thought to write will also take thought to understand. Undocumented or poorly documented code presents a problem for you when you have to review code you wrote in the past. You will have to wade through all of the functions and class methods etc. to find out what parameters they take, what they return and quite literally, what they do. If your code is procedural then than can even be a worse nightmare. If you for any reason need to have someone else modify your code I guarantee it will be a nightmare. I have had the displeasure of working with code that had absolutely no meaningful documentation. Tasks that would have taken me a few hours to accomplish had the code been properly commented took me days to finish. I have since vowed to NEVER take on any job involving poorly documented code. I would much rather write the entire thing from scratch.

7. Using User Input as an Argument in eval()

The function eval() is evil. eval() takes a string as an argument and allows that string to be executed as PHP code. That idea almost always appeals to newbie developers (as I must admit it did to me). This is no doubt a very powerful tool but it can be monumentously dangerous. Here’s a quote regarding its use from Rasmus Lerdorf, the creator of PHP:

If eval() is the answer, you’re almost certainly asking the wrong question.

I don’t absolutely agree. Personally I don’t use eval() at all but I have seen it used in some very creative ways. In every single one of those cases I have found an alternative that would give the same result. I just do not fancy the idea of dynamic code evaluation. If you must use the eval() function, never ever user user input as an argument for the function. You’d just be setting yourself up to be on the receiving end of a code injection exploit.

8. Recursive Inclusion

The days of having a laundry list of includes at the beginning of each and every file in your application are long past. If you are still running a version of PHP earlier than 5, upgrade and make use of the __autoload() magic function or the autoloading features of the Standard PHP Library (SPL).

9. Unnecessarily Wrapping Native Functions in Classes

Some programmers nowadays have adopted a ridiculous habit of wrapping almost every native PHP function they need to use in their application in a bloated class. I find this to be really annoying. This obsession with object oriented programming is all well and good, but procedural programming has its place. It has been my experience that writing tons of unnecessary classes actually slows down your application. Noticeably. Guys, come on! Stop writing wrappers for cURL!

10. Using die() to ‘Handle’ Errors

This one really grinds my gears because it is a result of sheer laziness. The die() function is a crude way to acknowledge errors and really should not even be considered a method of error handling because the error is not ‘handled’ at all it is simply acknowleged. Try at least using trigger_error() in conjunction with set_error_handler() if you’re not ready to make the step up to using exceptions but die() really needs to just, well, die.

11. Using Regex Functions for Basic String Manipulation.

This one is pretty straightforward. Yes, using regex is cool. Yes, using regex MIGHT make you seem like some kind of regex ninja. No you need not use them every single chance you get. Regular expressions are a very powerful tool which when used correctly, can accomplish some pretty amazing things as demonstrated in this excellent article. However, they are far slower than the native PHP string manipulation functions and tend to make your code more difficult to understand. So, if at all possible, try to substitute the ereg_* and preg_* family of functions with the str_* and ctype* family of string manipulation functions.

12. Not Closing Database Connections

We all want to open database connections but no-one wants to close them. If you have made a connection to a database then kindly close it when you’re done. Having it open is an additional overhead. Also avoid opening more than one connection to the same database in your scripts. Its better to reuse a connection than to keep opening and closing connections from a server resource perspective. To accomplish this, consider using a singleton approach.

13. Error Suppression Using The ‘@’ Symbol

Suppressing errors in general is not wise. Errors should be logged and handled gracefully. Using the error suppression operator in your code only makes debugging more difficult. Its very frustrating when your code does not do what it ought to do and you don’t receive any errors only to find hours later that had you not used the error suppression operator on some function x, you would have known that function x was causing an error. Bad practice. Stop it now.

14. Inappropriate Display of Errors

One of the signs that a website is being run by a complete amateur is seeing something like this after loading a page:

Fatal error: Call to undefined function: mysql_connect() in /home/httpd/mysite/php/myapp/include/db.inc on line 166

Users should never ever see errors like this. Once in a production environment, errors like these should be logged and the user sent a more user friendly message which divulges no details about the inner workings of your application. Error messages like the one above reveal intimate details of your code that not everyone should be privy to such as your directory structure. Conversely, in a development environment all errors should be displayed so you as the developer can quickly easily see where your code has gone wrong. Use the display_errors php.ini directive to control whether or not errors are displayed in your application.

15. Unquoted Array Keys

I very often come across code looking like this:

$myArray[boy] = 'harry';


This is bad coding practice. Array keys should always be enclosed in single quotes unless the key itself is a variable. If they aren’t, the PHP interpreter will consider the key a constant and search for a constant definition corresponding to that key. After not finding a constant definition, only then will the key be converted to a string. That just gives the interpreter more work to do. The above example written correctly would then become:

$myArray['boy'] = 'harry';

16. Using Uninitialized Variables

Due to the laid back nature of the PHP interpreter, it is possible to use a variable that has not been initialized as PHP will simply create the variable for you. Not only is this a result of laziness yet again, it makes for very confusing code. Always initialize your variables.

17. Placing Functions in Loop Declaration

Consider the following code:

for ($i = 0; $i < count($myArray); $i++)
{
//Code
}

This code will work, but what happens is that the function count() gets executed on each iteration of the loop. This in turn slows down the entire process. Try doing something like this instead:

$total = count($myArray);
for ($i = 0; $i < $total; $i++)
{
//Code
}


PHP is a very forgiving language and as a result it is easy to develop bad habits. As developers we should always strive to ensure that we are doing this the right way. Doing them the wrong way will always have some undesirable consequence.

Tagged as: bad practice, PHP, programming