Nothing too serious, but saying as you did ask for a critique, I'll
tell you things I'd have done differently. That DOES NOT necessarly
mean that my way is better than yours, but may be something you need
to take into consideration.
// #################### TEMPLATE ####################
function template($templatename)
{
// I can't imagine $templatecache being used outside of this
function, so I'd
// personally declare it static, instead of global, to avoid
poluting the namespace.
// Personally I generally avoid globals except for configuration variables.
global $templatecache, $setting;
// Personally I'd just if($templatecache[$templatename]), without the != ''
// What type of error reporting are you using? You may want to
consider simply
// checking if $templatecache contains a key called $templatename to avoid
// E_NOTICE, using array_key_exists(), or perhaps isset()
if ($templatecache[$templatename] != '')
{
$template = $templatecache[$templatename];
}
else
{
// consider the use file_get_contents(); I'm surprised at
your use of the error
// suppression operator. I know its good security practice not
to show errors, but
// logging them can often be helpful.
// adding or die('some useful, debugging error message'), may
help debugging.
// perhaps or die($setting['debugtemplate'] ? "template
$templatename failed to open" : "") would only display the error
whilst $setting['debugtemplate'] is set.
$fetchfile = @fopen('/templates/' . $templatename . '.tpl', 'r');
$template = @fread($fetchfile, filesize('/templates/' .
$templatename . '.tpl'));
@fclose($fetchfile);
$templatecache[$name] = $template;
// why are you adding slashes and immediately removing them again?
// perhaps you only want to slash the double quotes, and backslashes?
// $template = str_replace(array('"', '\'), array('\"', '\\'),
$template);
$template = addslashes($template);
$template = str_replace("\\'", "'", $template);
// just for the sake of making the code easier to read, I'd
if ($setting['debugtemplate'])
{
return "<!-- S:DEBUG:Template: $templatename
-->\n$template\n<!-- E:DEGUG:Template: $templatename -->";
}
}
return $template;
}
##########################################
Having that said I'd try to flatten down the if's a bit. Most of your
code is in an else block. Personally I'd probably do something like.
PLEASE REMEMBER THAT IM NOT SUGGESTING THAT MY WAY IS BETTER THAN
YOURS This is just food for thought.
function template($template_name){
$return_format = $setting['debug_template'] ? "<!-- debug_info
-->%s<!--end debug_info -->" : "%s";
if($template = $template_cache[$template_name]){
return sprintf($template_format, $template);
}
$template = read_template_from_file_and_process_it($template_name);
$template_cache = $template;
return sprintf($template_format, $template);
}
#####################################
One more thing: $template_name, or $TemplateName, or $templateName is
easier to read, than $templatename
On Sun, 02 Jan 2005 20:19:06 +0100, M. Sokolewicz <tularis@xxxxxxx> wrote:
> Sebastian wrote:
>
> > i have this small function for a template system (if you want to call it
> > that ;)
> >
> > my idea was having something simple to separate html code from php and i
> > think using something like smarty is too big for me and i really dont have
> > time to learn to use it. so i have this code here:
> >
> > http://www.cstrike-planet.com/function.phps
> >
> > i call this function which reads .tpl files which contain html and some
> > $vars, then in my pages use something like:
> >
> > eval('echo("' . template('home_index') . '");');
> >
> > unfortunaly my site has grown since and im not sure if this is causing an
> > extra overhead on the site..
> > is there a way to 'improve' this small function?
> >
> > thanks for any help.
> sure it has some overhead. However, you'd need a seriously large site to
> even notice it. The same way of templating is used by XMB (a message
> board system). It's quite widespread, and some of the boards are quite
> large, generating a lot of traffic, and are heavily used. However, such
> a relativly small thing doesn't have that big an impact on speed AFAICS :)
>
> --
> PHP General Mailing List (http://www.php.net/)
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>
--
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php
