As a freelance programmer, I have built quite a few web applications over the years, some pretty and some not-so-pretty. Before I got involved in PHP, I had worked mostly in Java, with its rigid and forced programming style. With PHP, of course, it's easy to get lazy and write sloppy code, and PHP will always perform well no matter what. Recently, however, I took over maintenance of a web application which set new lows for maintainability, quality, and structure. As I set about cleaning up this application, I took a few notes on what its most glaring problems were, and have devised a little quiz for you to take.

Most of this -- 99% of it - is old hat, or common sense, to many programmers. For others who are just starting, it may be useful. Certainly for the person who wrote the web application I took over, this would have been a valuable quiz.

The Barest Basics of Writing Decent Software:

Code Structure: Creating a directory structure, config file, naming standard, commenting, and formatting.
Error Handling: Checking every database query and properly displaying errors to the user.
Database Structure: Using a relational database properly.

Configuration and Directory Structure

One of the most outrageous problems I encountered in this application was the hard-coding of database username/passwords on every page throughout the site. If you ever need to change your database password, you would have to change every page. The simple solution, of course is to have a config file, which is included on every page. Give yourself 1 point if you have a proper config file.

In my applications, I create an "include" directory (it can be anywhere), and in that include directory, I place a file called "pre.php" that includes a config file, a database abstraction layer, and the template for page header/footer. It's then a simple matter to include "pre.php" on every page and inherit its functions.

Logical sub-sections of your application should be in their own directories. If you ever have more than 10-15 files in one directory, it's a sign you need to improve your directory structure. Give yourself 1 point if you have fewer than 15 files in each directory.

Naming Standards, Commenting and Formatting Your Code

It's valuable to have a naming standard for the functions you write in PHP. If you aren't using OO-style PHP programming, you can quickly run into name-space problems. I generally start my function names with the name of the sub-section I'm in. For instance, if I'm in the "bug tracker", all my files are in the "/bug/" directory and the functions start with "bug_". Bug-tracker-specific tables in the database would also start with "bug_" for clarity. Give yourself 1 point if you have a rational naming standard.

PHPDoc, which is based on JavaDoc, has been adopted as the standard for commenting and documenting functions in PHP. I use PHPDoc on more elaborate projects, but I generally just use short comments for tight-budget client work. Give yourself 2 points if you use PHPDoc or 1 point if you use standard comments.

I also encourage you to take pride in formatting your code. I always use plenty of braces ("{" or "}") in my code and parentheses wherever it makes sense. Braces are not strictly required all of the time, however the idea is to make your code absolutely clear and obvious to anyone who takes on its maintenance after you part ways with it. I also use appropriate vertical and horizontal (tabs) white space to make things clear. Here is an example:




<?php



if ($test) {



    //do something



} else {



    if ($foo == 123) {



        //operation here



    } else {



        //other operation



    }



}



?>

Notice the extra returns and generous use of tabs to space things out for clarity. Few people will struggle with understanding instantly what you are doing here. Give yourself 1 point if you are generous and consistent with your braces and white space.

Error Handling

The most common problem I see when reviewing code is a lack of error handling. What happens when no rows are returned from a database query? Most often, I see a while loop like this:




<?php



$res=db_query("SELECT * FROM users");



while ($row=db_fetch_array($res)) {



    //show output



}



?>

But what if no rows were selected? You have a hole in the middle of the page.

It's better to do this:




<?php



$res=db_query("SELECT * FROM users");



if (db_numrows($res)) {



    while ($row=db_fetch_array($res)) {



        //show output



    }



} else {



    print "No Users Found";



}



?>

Inside of functions, you should also handle all errors. The easiest thing to do is set a $feedback variable, and in your header/footer template, you always echo out that $feedback variable if it exists. This has worked well for me.




<?php



function my_function () {

    global $feedback;



    //code

    if ($error) {

        $feedback .= 'Could Not Perform Operation XXX';

        return false;

    } else {

        $feedback .= 'Operation XXX Successfully Performed';

        return true;

    }

}

?>

Then you simply call these functions like this:




<?php



if (my_function()) {



    //do something



} else {



    //handle error



}



?>

Of course, the same error checks should be performed on database updates, inserts, and deletes. Give yourself 1 point if you check all your database queries and provide proper feedback to the user.

Relational Database

Imagine writing an entire application and having only one database table! It's done all the time. If you have more than 10-15 columns in any table, it's almost certainly a bad design. If you ever find yourself with a table that looks like this:

Drivers
----------
Driverid int,
Lastname text,
Firstname text,
Job1_name text,
Job1_address text,
Job1_date text,
Job2_name text,
Job2_address text,
Job2_date text,
Job3 
...
States_available text

You get the idea. If you find yourself repeating columns like that, it means you should split into two tables (or more). If you are using a "text" field to store date entries, it's also a bad sign. Give yourself 1 point if you store dates and other data types in proper fields.

This table should have been split into two like this:

Drivers
--------------
Driverid int (autonumber),
Lastname text,
Firstname text,

Driver_jobs
--------------
driver_jobid int (autonumber),
driverid int, (relates to drivers table)
Name text,
Address text,
Start_date date

Another good sign of bad design is using explode()/implode() on your data before accessing the database. If you are storing a bunch of values in one big text field, say a big list of states, that's a horrible design. In the original "drivers" table above, the states this driver is available in is stored as a comma-separated list in one field. The proper solution is to create a third table like this:

Driver_states
----------------
driverid int, (relates to drivers table)
state_id char(2) (standard 2-digit state code)

Now that single awful table is broken into three properly-normalized database tables. This may sound like a pain to do, but later if you expand your application, you will appreciate having properly-formatted data.

Give yourself 1 point if you have properly-normalized database tables.

Results

If you have the maximum ten points, consider yourself a programming whiz, and kudos to you. If you have 5 or fewer points, please spend some time cleaning up your code. Chances are, you are making life difficult on yourself, your users, and the programming profession. If you have 6+ points, you probably fall into the majority of PHP programmers, and only a little discipline would be required to bring your code up to basic, decent standards.

If you have comments for more "signs of bad code" send me an email and I may include it in a future article.

Thanks,

Tim