Looping through DOM to modify elements

Question

I'm using this PHP to modify A elements in a DOM fragment (a vBulletin $post['message'] if that matters) so that links to external sites always open new tabs. This is actually the default in vBulletin but it's not always reliable. Anyway, the code works, but looking at the loop makes me think it could be done better. Any suggestions?

$as = $doc->getElementsByTagName('a');
for ( $i = $as->length; --$i >= 0; ) {
    $a = $as->item($i);
    $aParts = parse_url ( strtolower ( $a->getAttribute('href') ) );
    if ( $aParts['host'] !== $localDomain ) {
        $a->setAttribute ( 'title', @$a->getAttribute ('title') . " [external link to {$aParts['host']}]" );
        $a->setAttribute ( 'target', '_blank' );
    }   // if
} // for

Answer 1

To make it elegant clean code I would:

• make proper use of blank lines;
• abide by the coding standards defined by PHP-FIG;
• use the DOMNodeList object as an array so I can use foreach();
• be consistent with either single ' or double quotes ";
• use a guard clause.

Refactored code:

$aEls = $doc->getElementsByTagName('a');

foreach ($aEls as $a) {
    $url = parse_url(strtolower($a->getAttribute('href')));

    if ($url['host'] === $localDomain) {
        continue;
    }

    $title = $a->getAttribute('title') . ' [external link to ' . $url['host'] . ']';

    $a->setAttribute('title', $title);
    $a->setAttribute('target', '_blank');
}

(not tested)

Answer 2

DOM is a tree structure that consists of well defined objects (nodes) therefore I wouldn't use for loop.
Moreover, I don't know how for would influence the code if you need to do more radical actions like delete an element. I know for sure that if you delete an element while you are in a foreach loop you get an error and your changes are not committed.

There are 2 other easy ways to find elements in the DOM without using for or foreach:
- use Xpath - I would say the common and easiest approach because it doesn't need difficult coding
- use a recursive function to go through DOM; below how I would code this using an abstract class and closures. Another option is just to use a standard function in the global context but closures give some flexibility.

abstract class Utils
{
    static function loop(&$elem,$func)
    {
        $closure = function(&$el,$func){
            if($el->nodeType == 1 && $el->nodeName == "a"){
                $func = $fun->bindTo($el);
                $func($el);
                }
            else {
                if($el instanceof DOMNode && $el->hasChildNodes()){
                    $current = $el->firstChild;
                    while($current instanceof DOMNode){
                        self::loop($current,$func);
                        $current = $current->nextSibling;
                        }
                    }
                };
            };
        return $closure($elem,$func);
     }
}

Utils::loop($documentRootNode,function(&$a){
//do what you have to do here
//your A is either $a parameter or $this
//binding is not necessarily needed but is an option
});