Use DOM and XPath to make some changes in HTML document

Question

I intend to make some changes in an HTML document, like remove, replace, append some nodes.

I have several arrays with same structure like the following example

$patterns[] = array(
    'xpath'   => '/html/body/div/p',
    'insert'  => $new_a,
    'task'    => 'replace'
);

...

$patterns[] = array(
    'xpath'   => '/html/body/header',
    'insert'  => $new_b,
    'task'    => 'remove'
);

and I use the following snippet

$dom = new DOMDocument();

if ((empty($html) !== true) && ($dom->loadHTML($html) === true))
{
    $dom->formatOutput = true;

    // Use DomXPath
    $xpath = new DomXPath($dom);

    // Process Dom changes
    foreach ($patterns as $key => $value)
    {
        $xpath_results = $xpath->query($patterns[$key]['xpath']);

        if ($xpath_results->length) 
        {
            // Assign some values
            $xpath_results = $xpath_results->item(0);
            $task          = $patterns[$key]['task'];
            $replacement   = $patterns[$key]['insert'];

            if ($task != 'remove')
            {
                // Create the replacement
                $newNode = $dom->createDocumentFragment();
                $newNode->appendXML($replacement);
            }

            switch ($task)
            {
                case 'remove':
                    $xpath_results->parentNode->removeChild($xpath_results);
                    break;
                case 'replace':
                    $xpath_results->parentNode->replaceChild($newNode, $xpath_results);
                    break;
                case 'append':
                    $xpath_results->appendChild($newNode);
                    break;
                case 'prepend':
                    $xpath_results->parentNode->insertBefore($newNode, $xpath_results);
                    break;
            }
        }
    }

    // Save Dom 
    $html = $dom->saveXML($dom->documentElement);
}

I would like your expert comments about if this is done the right way. The foreach to loop through the array elements, the switch to choose what will be done and if the use of createDocumentFragment is appropriate in this case.

Answer 1

Your code seems to globally do the right job, using a right way, and I can't see how you could really improve performance.
I only noticed a few bad points:

1. Unless I missed some subtleness, there is a typo in the case 'append': branch: it should be using $xpath_results->**parentNode**,like others.
2. You didn't take in account a possible error condition at $newNode->appendXML($replacement);: it may return FALSE, so you should cancel the process in that case.
3. The same applies to a possible wrong value in the task index of a pattern: it should be taken in account through the default: option of the switch().

In the other hand I would suggest some improvements at the coding level itself, essentially for readability.

1. You're using foreach ($patterns as $key => $value), but then always write $patterns[$key][...] so you force PHP to a few more work, while $value is never used.
   I suggest to rather use foreach ($patterns as $value), then $value[...].
   (or even more readable foreach ($patterns as $pattern), then $pattern[...])
2. Still for readability I would add a separate variable to clearly distinguish the successive "avatars" of $xpath_results, which is first a DOMNodeList then a DOMNode.
   So you could change your DOMNode extraction to $node = $xpath_results->item(0);, and all following code becomes much more obvious.
3. Your // Assing some valuespart could be reduced to the $node assignation above: the two other variables are only used once so you could merely write $newNode->appendXML($pattern['insert']); and switch ($pattern['task']).

So here is my suggestion for the whole snippet after the remarks above:

if ($html) {
    $dom = new DOMDocument();
    if (!$dom->loadHTML($html)) {
        $error = 'Error while loading source HTML!';
    } else {
        $dom->formatOutput = true;
        // Use DomXPath
        $xpath = new DomXPath($dom);
        // Process Dom changes
        foreach ($patterns as $pattern) {
            $xpath_results = $xpath->query($pattern['xpath']);
            if ($xpath_results->length) {
                $node = $xpath_results->item(0);

                if ($task != 'remove') {
                    // Create the replacement
                    $newNode = $dom->createDocumentFragment();
                    if (!$newNode->appendXML($pattern['insert'])) {
                        $error = "Wrong insert pattern: $pattern['insert']!";
                        break;
                    }
                }
                if (!@$error) {
                    switch ($pattern['task']) {
                        case 'remove':
                            $node->parentNode->removeChild($node);
                            break;
                        case 'replace':
                            $node->parentNode->replaceChild($newNode, $node);
                            break;
                        case 'append':
                            $node->parentNode->appendChild($newNode);
                            break;
                        case 'prepend':
                            $node->parentNode->insertBefore($newNode, $node);
                            break;
                        default:
                            $error = "Wrong task pattern: $pattern['task']!";
                    }
                }
            }
        }
    }
    if (@$error) {
        // print or return $error...
    } else {
        // Save Dom
        $html = $dom->saveXML($dom->documentElement);
    }
}

For the error processing, I arbitrarily choosed one simple option among numerous different possiilities. You can notice:

1. The @$error form: it is a simple and clear way to allow using variables set only in certain cases without having defined them previously.
2. Still for readability, I changed the initial tests into more simple (and sufficient) forms: if ($html) and if (!dom->loadHTML($html)).