HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Tag manipulation using DOMDocument

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
manipulationusingtagdomdocument

Problem

On our main symfony2 application at work I have had to manipulate some outputted grid fields by wrapping them in a hyperlink tag. This happens in quite a few different controllers, so I made a method to do this to prevent code repetition.

I decided to do this using DOMDocument and construct the new hyperlink with that.

public function linkWrap($string, $route, array $route_params = [], $prefilter = null)
  {
    if(strlen($string) > 0)
    {
      $dom = new \DOMDocument();
      $new_hyper = $dom->createElement('a');
      $new_hyper->setAttribute('href', $this->generateUrl($route, $route_params).(isset($prefilter) ? '?'.htmlspecialchars($prefilter) : ''));
      $new_hyper->nodeValue = $string;

      return $dom->saveHTML($new_hyper);
    }

    return $string;
  }


The method takes a number of options the last of which is used only to append a prefilter query string for use with APYGridBundle.

My question is, does this cause a significantly larger overhead than just constructing the link in a more 'conventional' way?

As follows:

$link = "generateUrl('a_route', array('id' => 23)) . '?'.$prefilter">some text";


In addition, any tips on how any of could be improved would be appreciated.

Solution

Creating links using DOMDocument makes no sense to me.

I have created a simple benchmark for both methods.

Simple string concatenation is around 7 times faster on my computer:

Method Name Iterations Average Time Ops/second
------------------ ------------ -------------- -------------
simpleLink : [10,000 ] [0.0000013786793] [725,331.85764]
linkUseDomDocument: [10,000 ] [0.0000098103762] [101,932.89054]


You could reuse objects in (for example) static variables like this:

public function linkWrap2($string, $route, array $route_params = [], $prefilter = null)
{
static $dom = null, $new_hyper = null;

if(strlen($string) > 0)
{
if ($dom === null) {
$dom = new \DOMDocument();
$new_hyper = $dom->createElement('a');
}

$new_hyper->setAttribute('href', $this->generateUrl($route, $route_params).(isset($prefilter) ? '?'.htmlspecialchars($prefilter) : ''));
$new_hyper->nodeValue = $string;

return $dom->saveHTML($new_hyper);
}

return $string;
}


but it's only 30% faster.

My observations

Don't use htmlspecialchars in setAttribute value, because it will be encoded twice:

$new_hyper->setAttribute('href', $this->generateUrl($route, $route_params).(isset($prefilter) ? '?'.($prefilter) : ''));


However, you must use htmlspecialchars in simple concatenated strings:

$link = "generateUrl('a_route', array('id' => 23)) . '?'.htmlspecialchars($prefilter).'">some text';


Try to avoid custom query strings after a generated route URL. Instead of place value of $prefilter after the URL, you could place additional route params:

$this->generateUrl($route, array_merge($route_params, [ 'prefilter' => $prefilter ]));

Context

StackExchange Code Review Q#83845, answer score: 2

Revisions (0)

No revisions yet.