fabpot/Twig — Issue #572: Twig_Template::display should be refactored
Author: literal
Created: 2011-12-27 13:13:32+00:00
State: open

I just had a nice little debug session to find out why all globals are gone after upgrading to Twig 1.4.

Following Fabien's suggestion regarding issue #302 I have extended the Template class to implement `display()` without the `try`/`catch`. (In out setup we need the original exceptions thrown by objects accessed from within Twig templates.)

Now with Twig 1.4 the original `display` method contains some extra code to merge the globals into the template's context ,which was of course missing in my overriding implementation.

An obvious solution would be to copy the globals-related code into my implementation. But that is dirty and likely to break with future releases of Twig.

In my view the root cause of this problem is that the new `display` method does two things. No matter how short it may be, from a 'clean code' perspective it does two things on different levels of abstraction and these things should be factored out into separate methods:

/**
* {@inheritdoc}
*/
public function display(array $context, array $blocks = array())
{
$context = $this->mergeContextWithGlobals($context);
$this->displayWithErrorHandling($context, $blocks);
}

/**
* @param array $context
* @return array
*/
protected function mergeContextWithGlobals(array $context)
{
// we don't use array_merge as the context being generally
// bigger than globals, this code is faster.
foreach ($this->env->getGlobals() as $key => $value) {
if (!array_key_exists($key, $context)) {
$context[$key] = $value;
}
}
}

/**
* @param array $context
* @param array $blocks
*/
protected function displayWithErrorHandling(array $context, array $blocks)
{
try {
$this->doDisplay($context, $blocks);
} catch (Twig_Error $e) {
throw $e;
} catch (Exception $e) {
throw new Twig_Error_Runtime(
sprintf('An exception has been thrown during the rendering of a template ("%s").',
$e->getMessage()),
-1,
null,
$e
);
}
}

Some might argue this is over-explicit. But it's hard to deny that it makes the intent of the code clearer and it does allow to override the error handling without quirky duplication of code.