mezzio / mezzio-template

Template subcomponent for Mezzio
https://docs.mezzio.dev/mezzio/features/template/intro/
BSD 3-Clause "New" or "Revised" License
5 stars 7 forks source link

split `TemplateRendererInterface` in multiple interfaces #1

Open weierophinney opened 4 years ago

weierophinney commented 4 years ago

Following the Interface Segregation Principle I would like to propose to split the TemplateRendererInterface in three smaller interfaces

interface RendererInterface
{
    public function render($name, $params);
}

interface PathConfigurableRendererInterface
{
    public function addPath($path, $namespace);
    public function getPaths();
}

interface DefaultParameterRendererInterface
{
    public function addDefaultParam($templateName, $param, $value);
}

Then we could redefine TemplateRendererInterface as

interface TemplateRendererInterface extends RendererInterface, PathConfigurableRendererInterface, DefaultParameterRendererInterface

Often, when TemplateRendererInterface is required as a dependency, only the render method is actually used. With this proposal, we could use and type hint only on RendererInterface.

This could turn out useful if we want to implement a renderer which does not have the need to be configured with a set of paths to work properly


Originally posted by @marcosh at https://github.com/zendframework/zend-expressive-template/issues/14

weierophinney commented 4 years ago

Often, when TemplateRendererInterface is required as a dependency, only the render method is actually used. With this proposal, we could use and type hint only on RendererInterface.

What do you mean by often. In my code I load most of the templates from disk, so I need to add their location. I guess if you have templates stored in a database you could talk about often. You would need to load the template and pass it as a string to the renderer.

This could turn out useful if we want to implement a renderer which does not have the need to be configured with a set of paths to work properly

I think you can already use it without setting paths? I didn't try it yet though.


Originally posted by @geerteltink at https://github.com/zendframework/zend-expressive-template/issues/14#issuecomment-299819100

weierophinney commented 4 years ago

In my code I load most of the templates from disk, so I need to add their location. I guess if you have templates stored in a database you could talk about often. You would need to load the template and pass it as a string to the renderer.

I call Pareto here: seen this only once in my ZF projects, and it got phased out immediately as a build-time step/cache.

I think you can already use it without setting paths? I didn't try it yet though.

__construct is fine to pass in paths, and that's not interfaced, nor should be.


Originally posted by @Ocramius at https://github.com/zendframework/zend-expressive-template/issues/14#issuecomment-299819817

weierophinney commented 4 years ago

@xtreamwayz the point is was trying to make is not where you store your templates. My point is if a single interface should have all the responsibilities of being able to render a template, to manage the paths and to set default values.

Consider a more or less standard Action like the following:

class MyAction extends MiddlewareInterface
{
    /**
     * @var TemplateRendererInterface
     */
    private $renderer;

    public function __construct(TemplateRendererInterface $renderer)
    {
        $this->renderer = $renderer;
    }

    public function process(Request $request, Delegate $delegate)
    {
        return new HtmlResponse($this->renderer->render('templateName'));
    }
}

Only the render method is used and I doubt someone is going to use addPath or getPaths here. Therefore it seems more correct to me to type hint the renderer on an interface that has only the render method.

As @Ocramius said, that renderer could have been configured with arguments received in its contructor, or in some other way. Still, this detail should not emerge in the MyAction class


Originally posted by @marcosh at https://github.com/zendframework/zend-expressive-template/issues/14#issuecomment-299835263