HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Template system with master layout

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
templatewithsystemlayoutmaster

Problem

Here is my template code

```
templateFile = self::$path_dir . self::$templateFolder . '/' . $file . '.phtml';
$this->templateVars = $args;
}

/**
* Magic method to get the array element
* @param string $name template variable name to fetch
* @return mixed
*/
public function __get($name) {
if (array_key_exists($name, $this->templateVars)) {
return $this->templateVars[$name];
} else {
return '';
}
}

/**
* Magic method to set template var
* @param string $name
* @param mixed $value
*/
public function __set($name, $value) {
$this->templateVars[$name] = $value;
}

/**
* output the template to a string
* @return string
*/
public function __toString() {
ob_start();
include $this->templateFile;
return ob_get_clean();
}

/**
* Function to initialize the template
* @param string $templatePath Template Path
*/
public static function setTemplate($templatePath) {
self::$templateFolder = $templatePath;
}

/**
* Get current template path
* @return string
*/
public static function getTemplate() {
return self::$templateFolder;
}

/**
* Get current template path from root
* It will print the path. To be used in image and file paths
*/
public static function getPath() {
echo self::$path_web . self::$templateFolder . '/';
}

/**
* Set Master Template for project
*
* @param string $masterTemplate master template file name from template folder
*/
public static function setMaster($masterTemplate) {

self::$masterTemplate = $masterTemplate;

self::$placeholder = new Placeholder();
}

/**
* Set the Paths for root directory, and URL based path
* @param string $PATH_WEB URL Path e.g. http://localhost/project1/abc/
* @param string $PATH_DIR Physical path e.g. /home/userA/www/p

Solution

A lot of these suggestions are just that, suggestions. I have little to add for improvement except for aesthetics or readability. So here it goes.

Class Properties

Cool thing about class properties is that they can be combined when initially declaring them. A lot of people don't like doing it because when they see examples of it its always shown as one long string. However, it doesn't all have to be on the same line. Unless you are planning on commenting your variables, PHP is flexible and will allow you to pretty it up with as much, or as little, white space as you like. This is how I like initially declaring my properties, but as I said, this is just a suggestion/preference, it is up to you.

private
    $templateVars,
    $templateFile
;
protected static
    $templateFolder,
    $masterTemplate,
    $masterInstance,
    $placeholder
;


  • DocBlock commented variables have to be declared separately. You don't have to comment your variables, I don't, but figured I'd mention this for completion.



Constants

This is the only real problem I see. Constants PATH_WEB and PATH_DIR came from no where and I can't find them defined anywhere else. Your models should not be dependent upon another source to work, else you risk fatal errors should that source ever become unavailable or changes. Models should always work error free by themselves. This rule isn't as important for views or controllers since they are already so dependent on other files, but it still sort of applies for them to. I wouldn't set a constant in a config file, TEMPLATE_PATH, to be used in a controller or view, instead I would set it in the controller so that there was one less dependency. In fact, I would probably do away with the config file all together and just throw that all into the controller. The rule of thumb here: set it where you need it.

Method Organization and Improvements

This is just a preference, but one I think most would agree with. I like to move all my magic methods __get(), __set(), __construct(), etc... to the top of my classes. This makes them easier to find because they will always be in the same place. The way you have them now, they are just kind of in the middle. Not that it applies here because you don't seem to be using private/protected methods, but I also separate my methods by type (private/protected or public) for similar reasons. The final thing I do is visually separate these sections using comment blocks, which makes it easier to scroll long classes to find a specific section.

Why did you make your construct method chainable? Never tried it myself because I never thought I'd be able to do it cleanly so I don't even know if it is possible to chain it. Never used namespaces before either, but it looks like it can be done cleanly enough with them, I'll have to take a look at that. You don't appear to be chaining it anyways though, so I would just remove the return from your construct method.

Why did you create your own tostring method when you are not doing anything with the overloaded magic tostring method? Just combine the two and stop manually calling the method. Just echo the class instance, thats the whole reason for it. Also, to keep with the accepted standard, methods should only be prefixed by a single underscore "_" if they are private.

Final Thoughts

Now, my brain started shutting down when I looked at your usage sample, so this review is not 100%, but it should be a good 90% at the very least. That has nothing to do with your coding at all, I'm just tired and my brain doesn't want to do the mental gymnastics required to link everything in my head right now. I'll try and come back to this later if I see anything with your usage sample.

Code Snippets

private
    $templateVars,
    $templateFile
;
protected static
    $templateFolder,
    $masterTemplate,
    $masterInstance,
    $placeholder
;

Context

StackExchange Code Review Q#11551, answer score: 3

Revisions (0)

No revisions yet.