Skip to content
This repository has been archived by the owner on Jan 21, 2020. It is now read-only.

split TemplateRendererInterface in multiple interfaces #14

Open
marcosh opened this issue May 5, 2017 · 4 comments
Open

split TemplateRendererInterface in multiple interfaces #14

marcosh opened this issue May 5, 2017 · 4 comments

Comments

@marcosh
Copy link

marcosh commented May 5, 2017

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

@geerteltink
Copy link
Member

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.

@Ocramius
Copy link
Member

Ocramius commented May 8, 2017

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.

@marcosh
Copy link
Author

marcosh commented May 8, 2017

@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

@weierophinney
Copy link
Member

This repository has been closed and moved to mezzio/mezzio-template; a new issue has been opened at mezzio/mezzio-template#1.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants