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

Is this HTML Helper good or bad?

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

Problem

I have written this HTML static class which may categorize as a helper (not really sure what the definition of helper is). Anyways, I was just wondering if it was a smart idea to call a bunch of PHP functions instead of using raw markup because it just doesn't seem right to me.

One of the things that I'm positive about is that as output it creates a long string instead of many lines with tabs and line breaks (i.e. it is automatically minified). However, there's still this thing inside that's bugging me about these function calls because each element is a product of a function call, and this means that there will be a lot of calls, as opposed to a simple string output in ordinary case of raw markup.

This is an example usage:

echo \HTML::html(
    \HTML::head(
        \HTML::title('Hello')   
    )
    .
    \HTML::body(
        \HTML::h1('Hello world').
        \HTML::p('example')
    )
);


And the result:

HelloHello worldexample


In the actual usage I will not have concatenating strings like between head and body. I use a template engine and that's it's job, however now for the sake of simplicity I just use a concatenation.

The actual class itself:

```
/**
*
* Author php_nub_qq at stackoverflow.com
*
**/

final class HTML {
private function __construct() {

}

public static function __callStatic($name, $arguments) {
$content = isset($arguments[0]) ? $arguments[0] : null;
$attributes = isset($arguments[1]) ? $arguments[1] : array();

return ''.$content.'';
}

public static function script($src, array $attributes = array(), $defer = true){
$attributes['type'] = 'text/javascript';
$attributes['src'] = strstr($src, '://') ? $src : SITE_URL . $src;
if($defer){
$attributes['defer'] = 'defer';
}

return '';
}

public static function stylesheet($src, array $attributes = array()){
$attributes['type'] = 'text/css';
$attributes['href'

Solution

The general idea


is it good or bad?

I would say it's bad. But let's look at the pros and cons.

Advantages:

  • Decoupling: your content is decoupled from the actual HTML. If HTML ever changes, you only have to change your HTML class, and all the code that uses it continues to be correct. You could also easily print your content in a different format than HTML (well, maybe not easily, but it would certainly be possible).



  • Fast to Use: it might be faster to use than writing HTML yourself.



  • Minify: it can produce already minified HTML.



  • Correctness: You can verify that all HTML is correct (no missing closing tags, etc).



But: Do you really need 1.? I would guess not. And with a decent IDE, is it really slower to write HTML yourself? I wouldn't think so, so 2. is out as well. And 3.: You can just use a minifier, which will probably be better at what it does. And allows you the option of turning it off for debug purposes. Point 4. is the only one I would consider to be a real advantage (but then you would have to make it a priority to produce valid HTML).

Disadvantages:

  • Speed: it will always be slower than native HTML.



  • Complexity: native HTML will always work (it might not validate, but it will display something). You have to extensively test your class, and still, you might overlook something.



  • Usability: People know how HTML works, but they don't know your class, so they have to invest time in it.



In the end, you have to decide for yourself, but I think that the disadvantages outweigh the advantages for general HTML (for some elements, such as forms, a class that can generate them is indeed very helpful).

Your Code

HTML and Input

Your HTML class is quite coupled with your Input class. This makes re-using it a bit difficult, and it also leads to your HTML class doing more than just generating HTML. It now also contains program logic, such as displaying errors or inserting CSRF tokens).

Repeated Code

You have this code strstr($src, '://') ? $src : SITE_URL . $src; two times, and effectively three times if you count strpos($url, '://') === false ? SITE_URL.$url : $url;. You should extract this code to its own function.

input function

Your input function is too complex, try to think of a way to make it simpler. If you cannot think of a good way, extract code to its own function (the code at the beginning could go to createErrorDiv, and each of the cases could get its own function as well).

Misc

  • I would prefer to surround . with whitespace, I think it makes the code more readable (same for { and }).

Context

StackExchange Code Review Q#65268, answer score: 8

Revisions (0)

No revisions yet.