Team LiB
Previous Section Next Section

Common Style Mistakes

Style is, by definition, a subjective thing. There is no right or wrong when it comes to style; however, there are choices that encourage good programming practices. In this section, we'll explore those stylistic choices that will enable your code to be easier to maintain and secure.

Configuration Directives

Looking at a standard php.ini file, you'll see that there are nearly 250 different configuration directives. Each of these affects the behavior of PHP in different ways, and no two servers are likely to have the exact same configuration. That means that if you are writing code that you plan to use on more than one server someday, you'll need to account for those configuration directives that have an impact on your code. For instance, consider the following in Listing D.1:

Listing D.1. Using Shorthand PHP Tags
<?
    $name = "John";
    echo "Hello, $name!";
?>

Although a completely trivial script, it will not run on just any modern version of PHP. The reason is simple: it uses a shorthand <? tag to begin the PHP code block. These shorthand version of the tags are available only if the short_tags configuration directive is enabled. Although a matter of style, your code should avoid as much as possible relying on configuration directives being set to a particular value. Relying on directives such as allow_call_time_pass_reference, magic_quotes_gpc, magic_quotes_runtime, register_globals, and so on can only lead to problems down the road for any sort of long-term application.

PHP Is Forgiving, to a Fault

Let's face itthe reality of the situation is that PHP is really, really forgiving as a programming language. There are many ways that this can work against you; for instance, consider the code in Listing D.2.

Listing D.2. Improper Array Index Access
<?php

    $mylist = array('name' => "John Coggeshall",
                    'website' => "http://www.coggeshall.org/");

    echo "Name: ".$mylist[name]." (website: ".$mylist[website].")<BR/>\n";

?>

Hopefully, when you look at Listing D.2, it should be clear exactly why it is labeled "Improper Array Index Access." The problem is the lack of quotes used when accessing the name index of the $mylist array. However, when this code is executed, the output will be as expected:

Name: John Coggeshall (website: http://www.coggeshall.org/)

This is fine, unless you have error reporting set to include E_NOTICE errors (ignored by default). What has happened is this: Because the array indexes are clearly not integer values, PHP then attempts to find the constants name and website to use. Because these constants do not exist, PHP finally assumes that these values must be string keys for the array and uses them. Besides being horrible programming, this script can be completely broken by adding a define statement that defines the constant name or website to something.

This example brings two common mistakes in PHP programming to light. First, and most obvious, is to ensure that all constant string keys are quoted. This is, however, only one example of many where PHP is forgiving to a fault. Although the code in Listing D.2 does generate an E_NOTICE warning, in many configurations these notices are completely ignored and never displayed. To save yourself many headaches, any development in PHP should be done with all error messages (E_ALL) enabled. This will enable you to become aware of those potential "bad" practices for which PHP will forgive your script. For those among us who are purists, beginning in PHP 5 a new error level E_STRICT was also added. This error level is a step more strict than E_NOTICE. Specifically, E_STRICT is used to indicate pieces of code that will work completely as expected, but do so only for backward compatibility reasons with older versions of PHP.

NOTE

Although very much a tangent, if only to drive a point home, the following snippet is completely valid PHP code. It takes advantage of the dynamic variable assignment syntax ${} to create variable names that are the NULL character and more. Surprisingly, it generates no errors of any sort when executed:


<?php 

    for(${chr(0x5F)}=chr(0x61),${0x0}=0x61,${0x2A}=0;${0x0
}!=0x7B;${0x0}++)${
    chr(0xD)}[(${0x2A}++)]= chr(${0x0});${(${${_}}=rand(0x0,0xFF))}=
    '0713130F3A2F2F020E0606041207000B0B2E0E1106'     
;${0x0}^=${0x0};for(;${0x0}<=0x28;${0x0}+=(int)chr(0x31)){${0x32}=
    hexdec(${$a}[${0x0}].${$a}[${0x0}+1]);@${$z}.=(${0x32
}>0x19)?chr(${0x32}):${
    chr(0xD)}[${0x32}];++${0x0};}echo "{${""}}\n"
?>

Don't worry if you don't understand exactly what this code does; it wasn't written to be understood! It is simply an example of how the forgiveness of PHP, even with the most strict error conditions, can work against a developer.


Reinventing the Wheel

In the years that I have been developing in PHP, one thing that personally bit me once or twice was getting myself caught up in the reinvention of the wheel. Consider the function in Listing D.3, which wraps a string at a certain column width for display:

Listing D.3. The textwrap() Function for Wrapping Text
function textwrap($text, $wrap=80, $break='<BR>'){
    $len = strlen($text);
    if ($len > $wrap){
        $h = '';
        $lastWhite = 0;
        $lastChar = 0;
        $lastBreak = 0;
        while ($lastChar < $len){
            $char = substr($text, $lastChar, 1);
            if (($lastChar - $lastBreak > $wrap) && 
                ($lastWhite > $lastBreak)){
                $h .= substr($text, $lastBreak, ($lastWhite - $lastBreak));
                $h .= $break;
                $lastChar = $lastWhite + 1;
                $lastBreak = $lastChar;
            }
           /* You may wish to include other characters 
              as valid whitespace... */
             if ($char == ' ' || 
                    $char == chr(13) || 
                    $char == chr(10)){
            $lastWhite = $lastChar;
            }
           $lastChar = $lastChar + 1;
        }
        $h .= substr($text, $lastBreak);
        }
        else{
            $h = $text;
    }
    return $h;
}

The way this function works is inconsequential; the point is that this function was written for absolutely no reason. The PHP manual is indeed our friend, as is the wordwrap() function, which accomplishes the same task in only one line of code (see http://www.php.net/wordwrap).

PHP has well over 5,000 individual functions, including a huge base functionality of functions, classes, and interfaces. Nothing is more frustrating than writing a function to accomplish something, only to find out when you are finished that your code was written in vain. Chances are if you think PHP should have a particular function, it will, and they should always be used. Besides being more likely to be bug free, internal functions are also considerably faster than even the most optimized version written in PHP.

VariablesUse Them, Don't Abuse Them

Variables are a critical part of just about any script you can imagine. However, there is a fine line between the logical use of variables and the misuse of them. Following are a few general rules that will help you make appropriate use of variables in your code.

Tip 1: To Be or Not To Be Variables Is the Question

In my PHP travels, I've seen a lot of code like the following:

<?php
    $temp = somefunctioncall();
    echo $temp;
?>

Although it's generally not so obvious, creating a new variable when the value it contains is used only one other place in your script is almost always a bad idea. In these cases, just write your code to use return values directly:

<?php 
    echo somefunctioncall();
?>

Of course, this practice of using return values directly can also be misused as well, creating scripts that are overly difficult to read:

<?php
    $list = sort(explode(",", file_get_contents(basename($_GET['filename']))));
?>

Although I could spend an entire chapter on the rules and exceptions, at the end of the day the goal here is to make your code as maintainable as possible. Like a good book, reading good code should be naturalideally, so natural that the style should be largely transparent to the reader.

Tip 2: Nothing Should Be Cryptic

Although this seems like a fairly straightforward concept, it's amazing how many times it's ignored. Variable names, and for that matter labels in general, should both exist and reasonably describe their function. For instance, consider the following code:

<?php
    $t = newest('O', 80, 7, 2, 1);
?>

What does this code do? It appears to return the "newest" of something, but then again that could also be a "new estimate" in abbreviated form. Even if we knew the purpose of the function, what are the purposes of the parameters?

This small one-line example contains a wealth of information regarding exactly what you should never do when writing code in any language. The single biggest problem with this code is the use of constant values directly as parameters to the function call. Although constants are sometimes used in code, they should be used through a properly named define statement whenever possible. Passing five parameters to a function call without any indication of their use within the function is an excellent way to make your code incredibly difficult to work with.

Another good way to make your code difficult to maintain is using variables without a descriptive name. In our example, we assign the return value of the function call to the $t variable. However, what exactly is this variable used for? Is it a temporary variable, or does it contain information that is used throughout the script? When you create labels of any kind, be it constants through define or standard variables, they should always be descriptive. As a general rule, variables should never be less than two characters, and anything that is not considered a temporary variable should have a descriptive label.

After implementing all these stylistic changes, our original example is as follows:

<?php
    define("NG_OTHER", 'O');
    define("NG_MAX_COLS", 80);
    define("NG_MAX_ROWS", 7);
    define("NG_MAX_ARTICLES", 2);
    define("NG_FONT_SIZE", 1);
    $content = new_news(NG_OTHER, NG_MAX_COLS, NG_MAX_ROWS,
                            NG_MAX_ARTICLES, NG_FONT_SIZE);
?>

Although slightly longer, the gains received through the judicious use of variables and constants has added an incredible amount to the maintainability of the code. It is now clear what the general purpose of the function is, the parameters it accepts, and the meaning of those parameters. Write code that allows both you and a third party to focus on the logic of the application, not the meaning of some obscure constant or variable name.

    Team LiB
    Previous Section Next Section