Kleine PHP Dinge die man nicht machen sollte

Kommentieren Feb 03 2010 .txt, .json, .md

Es sind nicht wirklich neue und wichtige Dinge, aber solche die man auf jedenfall nicht mehr machen sollte. Manche lassen sich allgemein auf jeden Art von Programmier/Scrip Sprachen anwenden und nicht nur auf PHP.

PHP mistakes, misconceptions, bad practices and blatant no nos. http://www.sellmix.com/blog/php/php-mistakes-bad-practices-no-nos/

PHP mistakes, misconceptions, bad practices and blatant no nos.

Go to any PHP help forum and you’ll be sure to see many bad practises, innocent misconceptions and blatant no nos. None of us are perfect and none of us started off writing great code. We have all had that moment where we looked back on a script that we wrote years ago and thought “what the hell was I thinking?”. But it is a process. You make mistakes, you learn from those mistakes and then you move on. From my experiences of modifying other people’s code, helping people on PHP help forums and making my own (many) mistakes, I hereby present this list:

1: The PDO object does NOT necessarily make your database queries run faster.

The PDO is a data abstraction layer. It allows developers to create web applications that can work with different kinds of database management systems. If you’re not sure what DBMS (MySQL, Oracle etc) your web app is going to be using, you should use the PDO object. If you’re wanting to create a web app that can run on the majority of Database Management Systems, you should use the PDO object. However, don’t make the mistake in thinking that the PDO object is something that will make your queries faster – it won’t. The PDO object by default does not use buffered queries. This is because some DBMS’ do not support buffered queries.

2: Why are you using mysql_fetch_array?

Firstly, I’m not saying that using the mysql_fetch_array function is wrong. No, I’m just pointing out the fact that majority of beginner PHP programmers don’t understand what the function does (neither did I for some time). For example, you’ll often see code-pieces like this:

while($row = mysql_fetch_array($result)){
   echo $row['column_name'];

The thing is – mysql_fetch_array returns not only an associative array, but it also returns an array with numbered indices. Therefore, if you’re only going to be working from an associative array, you might aswell just use mysql_fetch_assoc() instead.

while($row = mysql_fetch_assoc($result)){
   echo $row['column_name'];

Or you can specifically tell mysql_fetch_array to return only an associative array:

while($row = mysql_fetch_array($result, MYSQL_ASSOC)){
   echo $row['column_name'];

The main lesson to be learned here is that you should never use a function without fully understanding what it does. Although, mysql_fetch_array isn’t significantly slower than mysql_fetch_assoc and mysql_fetch_row, there’s no point in using something that is slower than it’s alternative.

3: Indent, indent, indent!

When you don’t use proper indentation, not only do other people not want to look at your code, but you’re also making it far more difficult on yourself! What if, at a later stage, you have to return to that piece of code and modify it? Good luck to you and have a nice day!

4: Do not trust your users!

90% of beginner PHP programmers trust their users. I may have completely made that statistic up – but I’m pretty sure it would be close enough to that figure. Sure, the majority of the people who will use your web application will be nice people who would never dream of damaging your website. However, all it takes is one a!*hole with enough knowledge about web applications to ruin the entire show. If you’re asking the user to enter a number, check that what they entered is actually a number. If you’re asking a user to enter their email address, check to make sure that what they entered was actually an email address.

5: Brackets – use them.

    //do something

The above piece of code may work etc, but it makes your code far less readable. Instead, you should always try to use full parenthesis:

    //do something

It just makes your code easier on the eye and gives it some structure, that’s all.

6 – The often liberal assignment of variables onto one another

When you are constantly assigning variables onto one another, you’re not only using up memory – you’re also making it difficult to track any bugs that might pop up:

$tax_insurance = $total_tax;
$cost = $tax_insurance + $other_variable;
$another_cost = $cost + $other_variable;
$cost = $another_cost;
$goddamit = $another_cost;

7 – MySQL queries inside loops

This is a major no no. Not only do you risk incurring a large overhead on your database (depending on the loop, the query involved and how often that loop is executed) – if there is an error in your query, the loop will continue on and that error will therefore continue to happen. If you MUST run a query inside a loop, use prepared statements. MySQLi offers the use of prepared statements, as those the PDO object.

8 – Using addslashes instead of mysql_real_escape_string etc

When trying to make variables safe for insertion into a query, you’ll often find people using the function addslashes to escape dangerous characters. This is just wrong. mysql_real_escape_string was built on the underlying MySQL API, addslashes wasn’t. mysql_real_escape_string() takes into account the current charset of the connection, addslashes() doesn’t. Prepared statements are even better. However, if you’re not experienced enough to use prepared statements, mysql_real_escape_string should do just fine.

9 – White-listing is better than black-listing

When accepting data from users, it’s usually (read: always) a good idea to check if that data is correct. For example, if a user enters his or her email address into a form, we want to make sure that what they entered is actually an email address and not something else. If you let garbage into your system, your system is going to spit that garbage back out. Hence the old saying: Garbage in, garbage out. When checking this incoming data, it is always better to white-list things that are allowed into your system, rather than black-list things that are not allowed into your system. On PHP Help Forums, you’ll often see beginner users going through an awful lot of unnecessary bother. What they’ll do when checking data, is list out everything that the data shouldn’t be. Instead, they should be allowing data in that follows a certain rule. An example of black-listing is shown below:

$unclean_variable = $_POST['data'];
if($unclean_variable != 1 || $unclean_variable != 2 || $unclean_variable != 3 || $unclean_variable != 4){
   echo 'Incorrect data';

Why go through all that bother when you can do it this way?

$unclean_variable = (int) $_POST['data'];

if($unclean_variable > 0 && $unclean_variable < 5){
   echo 'Data is good';

A lot less code – a loss less work, and a lot less maintenance.

10 – Using or die(mysql_error());

Probably one of the most common bad practises out there. Instead of actually checking to see if a query failed or not and then taking action to correct the problem or re-route the flow of the application, the programmer will stick or die() at the end of their MySQL query. The result is that when one of their queries throws an error, the entire application grounds to a halt and a nasty MySQL error message is printed out to the screen. The user, who has no idea what MySQL even is, is left confused. When users get confused, they leave. Good job! Think about it. When an error happens on Facebook (a PHP driven website), you are shown an error message to tell you that something went wrong. The screen does not go blank.

11 – Superglobals being referred to inside class functions

Another common bad practise. You’ll often see class functions like this:

function my_function(){
   $var = $_POST['data'];

The problem with this kind of practise is that your application is now not as easy to maintain and modify. Your application is lacking in portability. What if you want to change the source of the data from $_POST[‘data’] to $_GET[‘data’]? Should you really have to go looking through lines and lines of code? Should you really have to go about modifying your classes?