PHP in Action

Conflict is not a bad thing. That is, if it leads eventually towards clarity and understanding rather than confusion and misunderstanding. And it's almost always better to have a conflict openly than to sweep it under the rug. Fear of conflict is the second of the five dysfunctions of a team.

And even though anger tends to cloud people's judgment, it need not be terribly destructive. If the angry person can manage to cool down and eventually keep an open mind in spite of it, it's not so bad. The problem is that people often get stuck trying to prove that they're right, placing blame and defending whatever position they started with at all costs.

One factor that exacerbates this is when different discussions get mixed up. When discussions get heated, you will often you get a discussion about the discussion mixed up with the original discussion. A debate about a technical matter may be conducted in parallel with an altercation that addresses the relationship between the participants. This is confusing in itself. Keeping these discussion apart will help maintain clarity.

In this vein, let me address the criticism I got in my latest blog post ("Comments considered harmful") regarding the appropriateness of the "considered harmful" idiom. This is not a technical issue, so let me give it a separate treatment.

A response I got was:

If ever there was a posting to which the "Considered Harmful" Essays Considered Harmful posting applied, it's this one.

To quote my own reply:

I have read that before. I fully understand the argument and agree with it in general terms. In particular, I agree with the idea that 'the writing of a "considered harmful" essay often serves to inflame whatever debate is in progress..'

In this case though, there seems to be no debate in progress in the first place. I am trying to make my point strongly enough that people will at least notice it.

If the rest of the blog post had been over the top, then I would probably have been shooting myself in the foot. I don't think it is, though.

I find the idea of "considered harmful considered harmful" valid, but over-generalized. Sometimes, a little conflict is needed to wake people up. Sometimes, you need to have two opposed camps for a while to make the difference of opinion clear before you can come to agreement.

When it comes down to it, it's an empirical question. I choose to measure the success, or lack of it, of my provocative title by the actual responses I get. Few of the commentators seem to have been derailed by the title. Some had specific technical objections, and that's fine even when I don't agree with the objections. Some asked me how specifically comments might be harmful. That also is totally relevant and, in fact, helpful.

Let me try to be even more precise. My aim in using this kind of title is not primarily to provoke, but to (over)simplify the message to make it easy to get some idea of the content by reading the title. If I had said "comments are not always useful", it would be so vague that anyone could embrace it. I would rather make it clear that I'm pushing a point of view that many are going to disagree with. Maybe I'll choose differently the next time, but I'm not promising anything.

There is too much old advice in PHP. A recent case comes from the PHP Advent calendar. Eli White is a strong believer in commenting code, including inline comments inside functions.

Unfortunately, he's at least 10 years too late. This used to be good advice, but not any more.

Up to a point, he's right. Making code as easy to understand as possible is essential and can save a huge amount of time later. And adding comments to unreadable code is better than leaving it the way it is.

There is a better way, though. Refactor your code so it's easy to understand even without comments. I'm just reading Robert C. Martin's recent bookClean Code. He is very clear on this point.

Clear and expressive code with few comments is far superior to cluttered and complex code with lots of comments. Rather than spend your time writing the comments that explain the mess you made, spend it cleaning that mess.

The principle is simple: if you have an inline comment in a method (or function), take the chunk of code that the comment refers to and extract it into a separate method. Give the method an intention-revealing name. Typically, you won't need the comment afterwards.

I'm sure we need an example. Here's a small excerpt from a method taken from Zend Framework.

    protected function _doUpdate()
    {
    ...
        /**
         * Execute cascading updates against dependent tables.
         * Do this only if primary key value(s) were changed.
         */
        if (count($pkDiffData) > 0) {
            $depTables = $this->_getTable()->getDependentTables();
            if (!empty($depTables)) {
                $db = $this->_getTable()->getAdapter();
                $pkNew = $this->_getPrimaryKey(true);
                $pkOld = $this->_getPrimaryKey(false);
                foreach ($depTables as $tableClass) {
                    try {
                        @Zend_Loader::loadClass($tableClass);
                    } catch (Zend_Exception $e) {
                        require_once 'Zend/Db/Table/Row/Exception.php';
                        throw new Zend_Db_Table_Row_Exception($e->getMessage());
                    }
                    $t = new $tableClass(array('db' => $db));
                    $t->_cascadeUpdate($this->getTableClass(), $pkOld, $pkNew);
                }
            }
        }
 

By changing some temporary varibles into instance variables and extracting a couple of methods, we can get code like this:

      if ($this->primaryKeyValuesHaveChanged() {
          $this->executeCascadingUpdatesOnDependentTables();
      }
 

Or even, by extracting a couple of classes, making objects out of two of the concepts involved, we might end up with something like this.

      if ($primaryKeys->valuesHaveChanged() {
          $dependentTables->executeCascadingUpdates();
      }
 

This uses most of the words from the original comment, but it's superior for several reasons. First and foremost, the section of code as a whole is clearer. Second, comments have a tendency to get out of sync with the code. By expressing the message in the code itself instead, we can avoid that. Third, the extraction itself has the positive side-effect of making it possible to test the extracted method in isolation.

Improving the code itself is harder than writing comments, but it's worth it. .

The core of your average web framework is a Front Controller. Front Controllers are commonly considered complex and esoteric. That's a myth. I sometimes brag that I can construct a Front Controller in 15 minutes. Actually, it's doesn't take quite that long. In PHP, a Front Controller can be simplified to just one line of code:

call_user_func($_REQUEST['action']);
 

Warning: Do not use this in a real application.

There is one and only one good reason why you shouldn't: it's totally insecure. Except for that, it's simple but perfectly viable. Your actions will be just plain functions, which is pretty simplistic and will be hard to handle if you build many of them. But it's a huge improvement over the average PHP script with no systematic handling of the PHP request.

If you want to do something more like the frameworks such as Zend Framework, you want your actions as methods grouped into classes (called Controllers in ZF).

Now you need two lines. The following script demonstrates this:

// Pretend we have an HTTP request
$_REQUEST = array('controller' => 'Contacts', 'action' => 'edit');
 
// Front controller
$controller = new $_REQUEST['controller'];
call_user_func(array($controller,$_REQUEST['action']));
 
// The action controller class
class Contacts {
    function edit() { 
        echo "ok\n";
    }
}
 

You can start with something like it (assuming that security has been taken care of) and add features as you need them.

In case you're wondering how to what exactly the security issues are, the most obvious problem is the fact that any user can run any method in any class that is available to this script. The way to avoid that append or prepend some fixed string to the class and the method name. For instance, in Zend Framework, the action name edit will activate the method editAction().

Beyond that, the security problems are the usual ones. You need to filter input, And in general you should beware of $_REQUEST, since it can also contain cookie data.

• Main
• Archives

• How code comments deteriorate
• How harmful is "harmful"?
• Comments considered harmful
• The one-line web framework
• How to test everything
• Zend_Test
• More beautiful code
• Beautiful code
• Refactoring is design
• Get links with XPath

• Application design [19]
• PHP [28]
• Refactoring [3]
• Frameworks [4]
• Testing [13]
• Ruby [3]
• Smidig2008 [1]
• Java [1]
• General [0]
• CSS [0]
• JavaScript/Ajax [0]
• Me [4]
• Vim [1]
• Humor [0]

• RSS 0.90
• RSS 1.0
• RSS 2.0
• Atom