Squirrel HackerI have come forward in time from the year 1986 to share this with you

I realise that I already posted something today, but this seemed like an emergency…

I thought maybe this was a serious post when I clicked on it, then when I got to number 2 on the list (I had skipped reading his post about how you should write an index page), I thought maybe he was joking, but at the end of it I realised that he was not.

1. How to Properly Create a Website Index Page
See my post about how to do it right.
[... snip]

$page = isset($_REQUEST['page']) ? $_REQUEST['page'] : 'home';
 
switch($page)
{
    case 'home':           break;
    case 'mail':           break;
    case 'contact':        break;
    default:
        $page = 'home';
}
 
include("$page.php");

[... snip]

Apparently this is the right way. I do like the single point of entry idea, and at least he is filtering the variables so you can’t load other php pages right? It really is too bad my $_COOKIE['page'] = ‘mail’… I wonder what the rest of the site is like. This also makes it a pain to add new pages, and causes a massive switch statement.

2. Use the Request Global Array to Grab Data
There is actually no reason to use $_GET and $_POST arrays to grab values. $_REQUEST, is another global array that fetches you either a get or form request. Therefore, it’s most times more convenient to use something like this to parse data…

No! This is wrong, the $_REQUEST array contains not only the $_POST and $_GET variable contents but it also contains the contents of the $_COOKIE array. The arrays are merged in the order described by your php.ini file, generally $_GET, $_POST, $_COOKIE but not always. So, use the $_POST variable when you mean for it to come from the $_POST variable, use the $_GET for get variables, and the $_COOKIE for things in the cookie. Don’t take the lazy way out.

3. Debugging PHP is About var_dump
If you’re looking for php debugging techniques, i have to say that var_dump is most times the way to go about it…

Wrong again. var_dump simply tells you what is in whatever object/array/whatever you are var_dumping. The xdebug extension is a much better alternative:

The Xdebug extension helps you debugging your script by providing a lot of valuable debug information. The debug information that Xdebug can provide includes the following:

* profiling information for PHP scripts
* code coverage analysis
* capabilities to debug your scripts interactively with a debug client

http://xdebug.org/

4. PHP Handles The Code Logic, Smarty Handles The Presentation
… Learn to use smarty as a template engine for your websites, it will pay off, i promise.

While I am not advocating combining “Code Logic” and “Presentation”, there are much better tools out there than Smarty. Smarty is a horrid piece of archaic spaghetti that had it’s purpose a long time ago, but no longer really does. Or you could learn to separate the presentation from the logic by using one of the myriads of frameworks out there. You could even go one better by separating it out to use the MVC pattern.

5. When You Absolutely Need Global Values, Create a Config File
… Doing it for database tables or database connection information is a good idea, but do not use global variables throughout your PHP code. Moreover, it is always a better idea to keep your global variables at a single config.php file.

Actually… this one isn’t bad, polluting the global namespace is something you should try to avoid, and keeping all of your config values in one place makes them easy to find.

6. If NOT Defined, Access Denied !
If you’re creating your pages the correct way, there will absolutely no reason for anybody to access any other php page other than index.php or home.php.

This goes back to #1, a horrible way to make an index page.

7. Create a Database Class
If you’re doing database programming (pretty common in PHP), it would be a very good idea to create a database class to handle any database management functions.

He then goes on to suggest you make a dbExec($query) function which calls $this->db->exec($query), and a sanitize($var1, $var2…) function which will not actually sanitize the input (it appears to just be making sure the input is numeric?). In his examples he is using the PEAR classes for his database abstraction, which already does this… so I am not sure what the point of putting a database abstraction ontop of a database abstraction is here. Also, use the escape functions that are already given to you by php (or the pear library so that your code is portable across database backends), do not write your own, you will get it wrong.

8. A php File Handles Input, a class.php File Handles Functionality
[...] The php file gets any input that we need and then redirects execution to a function residing to the class file. [...]

It almost seems like he is implementing a poorman’s MVC? I think? Mostly? A much better way would be to actually use an MVC framework (or to look at a tutorial online and see a much better way to do it).

9. Know Your SQL and Always Sanitize
Let me present you an example of a function that uses mySQL and sanitazes using the function seen on point #7

   private function getSentMessages($id)
   {
$this->util->sanitizeInput($id);
 
       $pm_table = $GLOBALS['config']['privateMsg'];
$users = $GLOBALS['config']['users'];
 
       $sql = "SELECT PM.*, USR.username as name_sender FROM $pm_table PM, $users USR
	    WHERE id_sender = '$id' AND sender_purge = FALSE AND USR.id = PM.id_receiver AND is_read = TRUE
	    ORDER BY date_sent DESC";
$result = $this->dbQueryAll($sql);
 
       return $result;
   }

The message is the right one. Sanitize your input. However, it would be awesome if he used some coding standards for naming his variables $user and $pm_table are both tables apparently. The well named function sanitizeInput should probably be renamed to checkInt or something similar since that is what it appears to do. He should also probably not be putting his variables straight in the string, to make it easier to change the query later on when he realises that he missed something. Don’t even get me started on the fact that this function is private and presumably extends his database abstraction class…

10. When You Need Just an Object, Use a Singleton Pattern
It happens pretty often in PHP that we just need a single object created one time and then used globally throughout our whole program. A good example of this is the smarty variable that has to be initialized once and then is used all over the place.
[..snip...]

function smartyObject()
{
    if ($GLOBALS['config']['SmartyObj'] == 0)
    {
        $smarty = new SmartyGame();
        $GLOBALS['config']['SmartyObj'] = $smarty;
    }
    else
        $smarty = $GLOBALS['config']['SmartyObj'];
    return $smarty;
}

[.../snip...]

The singleton pattern: you’re doing it wrong.

This is not actually a singleton pattern, while yes it lets you get the instance that you created earlier, you can still create the object by normal means, so I could have $GLOBALS['config']['SmartyObj'], $GLOBALS['config']['SmartyObj2'], $GLOBALS['config']['SmartyObj3'] which would all be instances of the smarty object, but they can all have different properties and values. If it were a singleton this would not be the case. The proper way of doing this is to use the pattern described in the php.net manual under Patterns Singleton. That way it is always the same object everywhere that you use it, and you do not have to muck around with the $GLOBALS array (which I am pretty sure is a code smell…).

Read his full post here. If you think I may have been too harsh, or not harsh enough, on him, leave a comment.