coral-erm / coral

CORAL ERM main repository
http://coral-erm.org/
Other
52 stars 64 forks source link

Separate views from logic with a template engine #78

Open tuxayo opened 8 years ago

tuxayo commented 8 years ago

Motivations

Separate business logic from presentation logic/views to:

We need a template engine because:

I only have ~50 hours of experience with PHP and Coral so the following initial work is expected to contain, errors, biases and irrelevances about

That being said, any template engine will allow to make huge progress toward the goals listed in the Motivations section. So there is no choice that is inherently bad. (except never choosing)

And the work to switch from one engine to another should in any case be lower than the work to separate the views from the business logic (for which a template engine will help a lot).

So don't be afraid that we could take decisions that might have terrible future consequences for the development of Coral. That's a perk of the current situation, we will necessarily remove more technical debt than increasing it.

Choice criteria

[1] https://www.quora.com/What-are-the-pros-and-cons-of-Smarty-versus-Twig-php-web-templates

[2] http://gm.zoomlab.it/2015/template-engines-i-moved-from-love-to-meh-for-plates/

[3] Search for "as little logic as possible" in http://www.sitecrafting.com/blog/top-5-php-template-engines/

[4] Someone on the #php IRC: I'd say using Mustache makes writing maintainable templates easier for developers with no template experience, IF they understand why Mustache is limited

[5] Someone on #php IRC: Separation of concerns. also better IDE support (for refactoring, find usages etc) and using the tool better designed for expressing business logic: a programming language (and yes, IMO display logic like "show the profile as yellow if the subscription is about to end" is business logic too) and that's where discipline comes in it's getting harder and harder to decide what should be "in" and what should be "out"

[6] I18n with Mustache.php

If I didn't missed something huge then the choice would be between Twig and Mustache.

To make the final choice I think if one or some of the Coral developers have experience with one of the two then it will make the difference. It will weight more in the balance than the difference found between Twig and Mustache (especially with the Caveats that alone create an error margin where the difference between Twig and Mustache fits)

So this is where we need feedback from those of us that already worked with template engines.

jsavell commented 8 years ago

I think a template engine is a secondary concern to adopting a true MVC pattern for the codebase. That would properly separate our data, logic, and presentation layers.

Shoehorning a template layer in now would only serve to make it more difficult to correct the overall codebase, later.

tuxayo commented 8 years ago

Shoehorning a template layer in now would only serve to make it more difficult to correct the overall codebase

Why? It would be just a tool to help separating the views and this could be the first step towards #80

jsavell commented 8 years ago

Without a clear definition of the MVC pattern goals and requirements, we'll just end up with a different set of problems to solve when we try to implement a MVC pattern.

The MVC pattern should be implemented in such a way that supporting a new view renderer is just a matter of implementing an interface. At that point, we'll be able to render CORAL views in HTML or JSON or XML with a few lines of code in a single class file and a configuration change.

We'll be able to support theming and skinning, easily.

We'll be able to drop in new template engines without entrenching them into the CORAL code and with significantly less commitment.

If we start integrating a specific template engine into CORAL without all that in place, we're likely to end up with a new dependency, tightly coupled between our controllers and views, which we'll then have to untangle and reimplement when we settle on the MVC design pattern.

That being said, https://github.com/Coral-erm/Coral/issues/80 wasn't opened at the time I made my comment, so if bringing in a template engine is done in the context of that Issue having well defined goals and requirements, then my concerns will have been addressed.

tuxayo commented 8 years ago

The MVC pattern should be implemented in such a way that supporting a new view renderer is just a matter of implementing an interface. At that point, we'll be able to render CORAL views in HTML or JSON or XML with a few lines of code in a single class file and a configuration change.

If I understand correctly we should use an intermediary layer (the view) to which the controller would pass the data to render. And the view would pass it to the appropriate renderer. (HTML template engine or JSON generator) So the actual calls to the template engine would not be scattered a over the controllers.

If we start integrating a specific template engine into CORAL without all that in place, we're likely to end up with a new dependency, tightly coupled between our controllers and views, which we'll then have to untangle and reimplement when we settle on the MVC design pattern.

We would pass the values to a template, is that such a high coupling? Would the intermediary layer proposed above be enough?

tuxayo commented 8 years ago

Updated first post with I18n consideration for Twig and Mustache

jsavell commented 8 years ago

It's the difference between our controllers being filled with lines like this:

$this->site->getViewRenderer()->registerViewVariable("widget",$widget); $this->setViewName('widgets.parts');

Or lines like this: $template = $twig->loadTemplate('widgets.parts.html'); echo $template->render(array('widget' => $widget));

In the first example, if we need to change ViewRenderers, or support more than one simultaneously (Twig,JSON, and XML, for example), it's no big deal. We just teach the $site class to instantiate the correct ViewRenderer implementation through Configuration settings or request parameters, long before the controller is even invoked: $site->setViewRenderer(new TwigViewRenderer()); or $site->setViewRenderer(new JSONViewRenderer()); $site->setViewRenderer(new XMLViewRenderer());

Nothing has to change in the controllers. They don't even know about it.

In the second example, though, our controllers know all about Twig. That's all they know how to do. If we want to change template engines, we have a lot of work to do in a lot of controllers. And we can't support XML or JSON globally. We have to fill our controllers with conditionals to test for when we should bypass Twig.

None of that is a knock on Twig. It was just an example to demonstrate the implementation concerns.

tuxayo commented 8 years ago

I see, the dependency injection would also allow to test only the controller or only the view, which is nice.

Would your proposed interface be enough or are there more things that need to be defined?

Also, as you posted a small example with Twig, does this mean that you have at least some experience with it?

tuxayo commented 8 years ago

Updated first post with possibility with Mustache to switch to client side rendering in the future.