Yet another reason smarty sucks

I’m doing cleanup today on a legacy project at work that uses smarty. Doing some fine tuning, and as part of that I’m resolving all notice level errors.

Y’know, since smarty is a preparser you think it would have the brains to do an isset() check on variables that it is about to echo or plug into conditional checks. Because that would result in clean code and take a nasty little detail of programming out of the designer’s hands so that wouldn’t have to worry their little heads about it… :nono:

Nah, vars in smarty need isset() checks too unless you want to raise notices.

Which brings me right back to my chief criticism of smarty which to this day it’s adherents can’t answer – what the Hell is the point?

There are two big reasons why I would use Smarty over alternative syntax:

  1. Template inheritence - yes you could re-implement that functionality using but why bother?

  2. If you need to allow clients the ability to update templates safely without running arbitrary code.

It promotes some questionable practices, such as formatting dates in the template layer, but as a tradeoff I think it also handles any concern for XSS?!? I’m not 100% certain on that last part but its one moire thing to consider.

Like any tool, there are times to use it and others not. Be frustrated with the previous programmer(s) not the library they chose - even that is subjective.

Cheers,
Alex

To be honest, it’s the underlying problem you hinted at which is the problem with smarty. isset() checks are needed because it requires variables to be pushed into the template, rather than having the ability to request them directly from another part of the system.

There are template engines that actually separate html from php without implementing logic in “template” files.

I hated smarty from the day I tried it and I was always wondering why would anyone use it since you’re forced to learn some weird syntax that’s so similar to php - so why not use php from the start?

To be honest, it’s the underlying problem you hinted at which is the problem with smarty. isset() checks are needed because it requires variables to be pushed into the template, rather than having the ability to request them directly from another part of the system.

Are you refering to a pull-based template engine as opposed to push? The problem there of course, is now templates form a dependency on the model, unless you are suggesting the template is the View, in which case I can see your point, but prefer to keep views separate from the templates.

IMO variables should be checked, sanitized and given defaults before the template layer receives them, thus making isset() a non-issue for me at least. But depending on your architectural choices this may or may not be a valid argument.

There are template engines that actually separate html from php without implementing logic in “template” files.

I hated smarty from the day I tried it and I was always wondering why would anyone use it since you’re forced to learn some weird syntax that’s so similar to php - so why not use php from the start?

Templates engines that separate HTML from PHP entirely are not doing anyone any favours. For one, it’s a moot concern, anything complex requires conditional logic to render. If it is not done in templates, it’s typically done in the Controller or Model and thus tightly coupling your application code to XHTML, HTML, XML or whatever your output may be for that project. This is ultimately one of the worst choices we can make as developers as the interface changes more frequently than anything else. Wther that be switching from XHTML to HTML5 or whatever.

That weird syntax, is IMO, a trivial counter argument. The docs are good enough anyone with basic computer skills should be capable of learning it. A few simple constructs in a very limited context.

In this case, the benefits out-weight the negatives by a long shot.

p.s-I’m not a Smarty fanboy, I have my issues with it too, but I’m just offering some experience, opinion, whatever you want to call it. I prefer to see everything obectively :slight_smile:

Cheers,
Alex

So, if I have quite a bit of HTML to mark up some content, you consider that a templating engine won’t do me any favor because I’ll have html at one place and code that repeats parts of that HTML and assigns variables to it at another place?
Of course that the controller will do the rendering, however I won’t have to fit all that nice HTML with javascript and what not inside it. I can keep it separate in a file that’s pure HTML, so anyone can edit it and even if they make mistakes such as not closing tags - it won’t cause any parse errors.

I strongly disagree with you that templating engines aren’t doing anyone favors, there are many many examples I can provide to prove how templating engine can save you time and you won’t get asked questions such as “what does parse error at line 239 mean? I just added some css to the page”.

That weird syntax, is IMO, a trivial counter argument. The docs are good enough anyone with basic computer skills should be capable of learning it. A few simple constructs in a very limited context.

Actually, no, it’s not trivial. Why would I be forced to learn syntax of Smarty? I don’t want to know yet another language solely for the purposes of templating.
I already know HTML. I already know PHP. I already know CSS. I already know JavaScript. And look, all the ingredients for creating a website are here! So on top of that, I would have to learn yet another thing that combines loops, conditions and really really weird syntax and naming conventions.

Why wouldn’t I just ditch it and forget about Smarty? It’s not like it separates anything, nor its markup is valid HTML code.

As I said, there are templating engines that separate html from PHP. And that IS a huge favor as you can quickly modify parts of that markup to achieve different output without touching php that takes care of pulling the data from the data source and sticking it to the html.

Smarty doesn’t do that, it just forces the user to learn yet another language, on top of everything else.
It’s pointless to use it, I’m just saying it’s better for someone who already decided to use Smarty to use pure PHP as it’s practically the same effort.

Just my 2 cents :slight_smile:

So, if I have quite a bit of HTML to mark up some content, you consider that a templating engine won’t do me any favor because I’ll have html at one place and code that repeats parts of that HTML and assigns variables to it at another place?

Tricky to answer because I am not sure what you are refering to exactly. For example, a CMS typically stores “content” as a combination of markup and textual content. This “content” is often injected into a master template. I use a markdown so no markup is involved.

On the contrary, if you are rendering a array/list into an HTML list in something other than a template - that is a bad practice. Looping over an array and conditionally displaying hilited records (or whatever) is not possible without logic, so if your template layer is ‘dumb’ and simply a search and replace approach, one is clearly performing this HTML translation in a layer other than template layer, which is a bad practice, IMO.

I strongly disagree with you that templating engines aren’t doing anyone favors, there are many many examples I can provide to prove how templating engine can save you time

I said template engines aren’t doing us favors? I assure you that was a typo or you misunderstood you. :slight_smile: My bad I meant the opposite.

Actually, no, it’s not trivial. Why would I be forced to learn syntax of Smarty

In the two instances I initially gave, Smarty would be required, or a similar template engine, PHP alternative syntax doesn’t really facilitate either without lots of hacking.

don’t want to know yet another language solely for the purposes of templating.

It’s hardly what I would consider another language. :stuck_out_tongue: but that is subjective I suppose. I am quite adept at learning new technologies as the situation requires. Each to his own I guess.

I already know JavaScript. And look, all the ingredients for creating a website are here! So on top of that, I would have to learn yet another thing that combines loops, conditions and really really weird syntax and naming conventions.

It’s no more weird or esoteric than joining a new project and having to re-learn another teams conventions, best practices, architecture, etc.

Why wouldn’t I just ditch it and forget about Smarty? It’s not like it separates anything, nor its markup is valid HTML code

Again refer to my first two points. As for invalid XHTML. Really? I thought they fixed that? In anycase, thats a valid argument and sucks, though I am sure it’s easy to hack a fix in the Smarty core/plugin to work around this issue.

Smarty doesn’t do that, it just forces the user to learn yet another language, on top of everything else.
It’s pointless to use it, I’m just saying it’s better for someone who already decided to use Smarty to use pure PHP as it’s practically the same effort.

It’s clear to me now I mis-communicated my intent. I am not saying templates are bad, I am saying that echo’ing HTML in a Controller or Model is bad practice. Any output should almost always be stored or handled in a template layer, regardless of whether Smarty, Twig, Savant or similar PHP alternative syntax is used.

Cheers,
Alex

take a look at this (actually very well-built) templating engine for php :wink:

Michael, reading from your posts you must be on a special crusade against smarty! :slight_smile:

Why don’t you set the error reporting level only for the templates to hide the notices? In the newest version of smarty you set it at smarty object initialization and it’s documented. I know there are flaws with smarty but regarding this one it looks like you didn’t do the research or read the docs.

Well, but is there such a requirement really? You could, for example, push only one object to smarty like $model and then you could request all needed variables in the templates through this object. There’s also nothing preventing anyone from calling static::methods() from the templates. When used it like this, I don’t think there’s much functional difference between smarty and a php template, smarty just provides simplified syntax. Or am I missing something more fundamental here?

Yes, I am. Smarty is bad on many levels, not the least of which is this - it is a template engine inside of a template engine. PHP is a template engine folks. While it has evolved into a full bore language, it’s roots as a template engine are readily apparent right at the fact that every PHP starts with <?php and if you don’t start with that you’ll be echoing content immediately…

Hmm… Like a template engine.

Building a template engine in a template engine is like getting a semi truck and then towing it around with a pickup truck. Bluntly, it’s stupid.

The goal of “separating PHP from HTML” is manifest ignorance and nothing more. PHP, especially PHP short tags, is no harder to read than smarty’s markup. I’m not against learning new markups either - but only when there’s a point to the effort. With smarty there’s not.

And I’ve yet to meet one designer who was smart enough to grok smarty and too stupid to handle PHP short tags. So why inflict yet another syntax on the team.

Up thread someone went so far as to say logic should be separated from templates. Ahem, no. Display logic has to go somewhere, and if you working in MVC you either place it the view or you’re breaking the pattern. That said, Templates are NOT the entire view. I suppose you could get all your display logic into support classes, but its asking for a lot of trouble and limited flexibility.

The goal of MVC is separation of concerns – logic types – not the separation of logic from the templates or logic from the view. It simply means that no business logic (model) or program control logic (controller) is to be in the view. Display logic, such as a loop to generate table rows, is fine.

As for tweaking error reporting to hide notices – Sometimes notices are important and clue you in to larger problems. I pride myself on having my production code be able to run at error_reporting(-1) without emitting an error. When they do occur I use an error handler to collate them into a json string that is attached to the outgoing html or js file and then sent by javascript to the console of firebug or chrome. That functionality gets crippled if there are oodles of insignificant isset errors flying about. Also, such loose coding is just plain lazy and establishes bad habits that come back to haunt you when you move to C or any other language that isn’t tolerant of on the fly var casting.

Building a template engine in a template engine is like getting a semi truck and then towing it around with a pickup truck. Bluntly, it’s stupid.

I disagree. It’s more analogous to building a more abstract language ontop of a lower level langauge, just as the evolution of every language to date has proceeded. :slight_smile:

The goal of “separating PHP from HTML” is manifest ignorance and nothing more. PHP, especially PHP short tags, is no harder to read than smarty’s markup. I’m not against learning new markups either - but only when there’s a point to the effort. With smarty there’s not.

  1. Template inheritence
  2. Client editable templates

And I’ve yet to meet one designer who was smart enough to grok smarty and too stupid to handle PHP short tags. So why inflict yet another syntax on the team.

If you don’t need either of the above two requirements, then using PHP alternative syntax is probably the better choice.

Cheers,
Alex

…and a very ugly one! Take it easy - the fact that you love php’s templating syntax doesn’t mean everybody else must. Some people think it’s clean and elegant to simply use {$var}. Don’t let’s talk about style preferences.

The goal of “separating PHP from HTML” is manifest ignorance and nothing more. […]
Up thread someone went so far as to say logic should be separated from templates. Ahem, no. Display logic has to go somewhere, and if you working in MVC you either place it the view or you’re breaking the pattern. That said, Templates are NOT the entire view. I suppose you could get all your display logic into support classes, but its asking for a lot of trouble and limited flexibility.

The goal of MVC is separation of concerns – logic types – not the separation of logic from the templates or logic from the view. It simply means that no business logic (model) or program control logic (controller) is to be in the view. Display logic, such as a loop to generate table rows, is fine.

That’s all very well said but what does it have to do with smarty? This applies to any template engine, even if you use php. Sure, you can misuse smarty and put display logic outside the template but the same applies to pure php. This is in no way an argument for “smarty sucks”.

As for tweaking error reporting to hide notices – Sometimes notices are important and clue you in to larger problems. I pride myself on having my production code be able to run at error_reporting(-1) without emitting an error. When they do occur I use an error handler to collate them into a json string that is attached to the outgoing html or js file and then sent by javascript to the console of firebug or chrome. That functionality gets crippled if there are oodles of insignificant isset errors flying about. Also, such loose coding is just plain lazy and establishes bad habits that come back to haunt you when you move to C or any other language that isn’t tolerant of on the fly var casting.

Can you make yourself clear - in your first post you said you didn’t want notice errors for smarty vars, now you say you want to see notice errors. So do you want to see them or not? Or maybe you want to only suppress notice errors for undefined smarty variables? You can do it easily with your own error handler.

Using isset() isn’t error suppression - it’s error resolution. There’s a big difference because the underlying engine isn’t asked to make any guesses.

Anyway, I hate smarty. I’ve made it clear and I’ll shut up again until the next time I have to work with it on the legacy projects here at work.

Up thread someone went so far as to say logic should be separated from templates. Ahem, no. Display logic has to go somewhere, and if you working in MVC you either place it the view or you’re breaking the pattern. That said, Templates are NOT the entire view. I suppose you could get all your display logic into support classes, but its asking for a lot of trouble and limited flexibility.

Display logic in templates is bad because it creates non-reusable code and by extension, maintainability issues. It’s the equivalent of inline CSS styles vs external stylesheets.

Consider this:


<?php
if ($user->isLoggedIn()) {
?>
Welcome back <?php echo $user->name; ?>
<?php
}
else {
?>
Welcome, Guest, Please login or register.
<?php
}
?>

vs:


<returning_user>
Welcome back <?php echo $user->name; ?>
</returning_user>

<unregistered_user>
Welcome, Guest, Please login or register.
</unregistered_user>

It’s the same, right? Not really. How many times may you do that if is logged in check? Potentially very many, across a lot of templates.

Consider the requirements change. Due to spam we’ve just introduced account verification and want to stop people from being able to do anything until they’re registered and been verified. We could change the isLoggedIn() check, but that’s rather hacky inst it?

Now I need to find every occurrence of if ($user->isLoggedIn()) in every template and replace it with if ($user->isLoggedIn() && $user->isVerified()).

Using marked up template sections, I adjust the logic in one place and it affects everything, everywhere. Sure there may be a couple of exceptions where logged in and unverified users can do something slightly difference but those exceptions are all I need to be concerned with.

Well, but is there such a requirement really? You could, for example, push only one object to smarty like $model and then you could request all needed variables in the templates through this object. There’s also nothing preventing anyone from calling static::methods() from the templates. When used it like this, I don’t think there’s much functional difference between smarty and a php template, smarty just provides simplified syntax. Or am I missing something more fundamental here?

Of course, but most people don’t use smarty in this way and implement a lot of pointless data-binding logic. Binding one variable to the template makes sense because it adds a flex point. Binding each variable introduces unnecessary binding logic.

Anyway, I hate smarty. I’ve made it clear and I’ll shut up again until the next time I have to work with it on the legacy projects here at work

LOL if you feel you have nothing more to argue then that is appropriate. However if you missed something then by all means keep going. I have switched gears on Smarty about a dozen times since it’s first release so I have many opinions about it.

As long as we all stay civil and professional there is little wrong in expressing ones concerns about anything.

Display logic in templates is bad because it creates non-reusable code and by extension, maintainability issues. It’s the equivalent of inline CSS styles vs external stylesheets.

Templates are probably the least reused aspect of an application, every client will want something tweaked in the UI, whether it’s a simple style or an extra column in a table, or percentage bar instead of raw values.

Let us be very clear as to what we mean by display logic. I find that many times what I consider display logic to actually be model logic. A pager for instance, the calculations performed to determine offsets, range of paged indexes, etc. This is not display logic and is indeed model logic, but of a special type, that of a pager not a business application.

My biggest concern with handling display logic in anything other than templates is the coupling it produces between an output type (HTML, CSS, XHTML, JS, XML, JSON, etc). If your view class does something like:

class MyView{
  render()
  {
    $buff = "<div>";
    $buff = "Render a table provided an array";
    $buff .= "</div>";

    return $buff;
  }
}

This is a bad practice, at least in the way I develop applications, due to the class being dependent on HTML it is not typically reusable across applications, but in one specific application at most.

The only time I have encountered a need to have display logic in a function or class was in the case of rendering a structured tree (sitemap) because my implementation used recursion which is typically not possible at template layer.


TomB: Having re-read your suggestion for using what one might call “conditional blocks”??? It’s an interesting solution. You need a mechanism to bind a block to conditional test, therefore removing all conditionals from the template.

All markup is kept in the template layer, which is my greatest concern?

Just curious but how do you bind template blocks to conditional tests?

What would the template look like if you were iterating over a recordset that was paginated 10 times with 10 results per page (100 in total) and you needed to conditionally hide/display records which the user has indicated they want removed and the state is stored in a SESSION (think a collapsible tree like control - so state is per user basis)???

Can you show me example code for the view/model and template?

Cheers,
Alex

Well I’d like to (try to, but it wont work) keep this post short, so without offering full documentation to my whole view layer, here’s what I’d do to get the above working.

My current code is perhaps overkill in most cases. I built it to offer maximum reusability and minimal client code at the expense of a large application codebase.

Now this could work without my template engine entirely and work with php as a template engine, the reason I use my template engine is not for this, really but more for the pagination type example below.


//Common user-related display logic, probably already defined somewhere.
abstract class UserViewModel extends ViewModel {
	public function getUser() {
		return $this->model->user->fromSession();	
	}
	
	public function isReturningUser() {
		//Here, as previous example I could change it to $this->model->user->isLoggedIn() && $this->model->user->isVerified();
		return $this->model->user->isLoggedIn();
	}
}

class WelcomeMessageViewModel extends UserViewModel {
	public function getTemplate(TemplateLoader $templateLoader) {
		return $templateLoader->load('welcomemessage.tpl');
	}
}

welcomemessage.tpl


<!-- visible when isReturningUser is true -->
<section:isReturningUser visible="true">
	Welcome back {model property="getUser()::name"}
</section:isReturningUser>

<!-- visible when isReturningUser is false -->
<section:isReturningUser visible="false">
	Welcome, Guest, Please login or register.
</section:isReturningUser>

And that would be it. Of course I have a top level “View” layer (defined by the framework, rather than each time…) which automatically maps the ViewModel to the template… the template can be reused with a different viewmodel (substituting the logic) or the viewmodel can use a different template (substituting the html).

It’s a one way data flow, the template pulls data from the viewmodel and the viewmodel pulls data from the domain model creating a very strict separation of concerns. I wont go into pagination in detail but this would be everything i’d need to do to implement it in my system as is:


class UserListViewModel extends ViewModel Implements PagedDataViewModel {
	//random variables settable by controller
	public $searchCriteria = 'Tom';
	public $page;
	
	public function getTemplate(TemplateLoader $templateLoader) {
		return $templateLoader->load('userlist.tpl');
	}
	
	public function find($limit, $offset) {
		//Ask domain model for related records
		return $this->model->user->find(array('name' => $this->searchCriteria), $this->limit, $this->offset);
	}
	
	public function getCurrentPage() {
		//Page currently being viewed
		return $this->page;
	}
	
	public function getRecordsPerPage() {
		//I want to show 10 records per page...
		return 10;
	}
	
	public function getTotalResults() {
		return $this->model->user->countFind(array('name' => $this->searchCriteria));
	}
}

userlist.tpl



<!-- The vars are optionally specified, the view defines some defaults -->
<section:pagelist numvar="num" classvar="class" classcurrent="currentpage" classpage="page" >
	<a href="userlist/{num}" class="{class}">{num}</a>	
</section:pagelist>

<!--  the search results, the name of these sections can be speficied by optional functions in the viewmodel -->
<section:result into="item">
	<div>
		<p>{item property="firstname"}</p>
		<p>{item property="lastname"}</p>
	</div>
</section:result>

In short the binding is done by the specified view.Here’s a simplified version of how that works (using hardcoded everything for the sake of simplicity), this is just part of the framework and not defined per project or each time pagination is used.


class PagedDataView extends View {
	public $sectionName = 'result';
	public $pageSection = 'pagelist';
	public $currentClass = 'current';
	
	public function output() {
		$perPage = $this->viewModel->getRecordsPerPage();
		$pageNo = $this->viewModel->getCurrentPage();
		if (empty($pageNo) || !is_numeric($pageNo) || $pageNo < 1) $pageNo = 1;

		$result = $this->viewModel->find($perPage, ($pageNo-1)*$perPage);
		
		$totalRecords = $this->viewModel->getTotalResults();
		$totalPages = ceil($totalRecords/$perPage);
		
		for ($i = 1; $i <= $totalPages; $i++) $this->template->appendSection($this->pageSection, array('num' => $i, 'class' => $i == $pageNo ? $this->currentClass : ''));
		
		
		
		$this->template->addHook($this->hook->objectSet($this->sectionName, $result));
	

		return parent::output();
	}
}

sorry, as expected this post got long :stuck_out_tongue: Now you’re going to ask about template hooks aren’t you?.. in brief they just match tag sections in the template and enable control of the contents.

Feel free to PM me, but this is way beyond the scope of the initial discussion.

99% of the time I find that smarty is used by cut and run programmers who leave spaghetti code. It’s a quick solution to getting something done. If you care to customize, get ready to run in circles. If I see smarty as a solution I usually begin looking elsewhere. I just saw this post highlighted and figured I’d share my opinion.

99% of the time I find that smarty is used by cut and run programmers who leave spaghetti code. It’s a quick solution to getting something done. If you care to customize, get ready to run in circles. If I see smarty as a solution I usually begin looking elsewhere. I just saw this post highlighted and figured I’d share my opinion.

Only because Smarty is a stepping stone for some developers when they realize that presentation logic should be separated from PHP logic. What usually happens there after is the developer discovers MVC and begin that journey.

Cheers,
Alex

I’m really excited about Smarty 3. It makes it even easier to separate the code from the design, which is really one of the main goals of a template system. There have been hundreds of templates made for our software over the years in Smarty (ex) and clearly it has worked. I’m sure there are other good systems out there too. In the end, what matters most is you have solid PHP code running things (securely!) and people who want to make design changes can have freedom to work. You can certainly implement Smarty badly. We have learned a few things over the years. But again, Smarty 3 is really cool, and lots of companies are adopting it (ex. [URL=“http://templates.phplinkdirectory.com/”]Elance). That means there are some very competent programmers using Smarty.

I have found Smarty a great system to use. It reduces my View code quite a bit and it’s easy to remember things. It makes my life easier. Not everyone who uses Smarty just uses it to whip together sloppy spaghetti code, I don’t know how that conclusion is drawn.