10
\$\begingroup\$

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:

<html><head><title>Hello</title></head><body><h1>Hello world</h1><p>example</p></body></html>

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 '<'.$name.self::parseAttributes($attributes).'>'.$content.'</'.$name.'>';
    }

    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 '<script'.self::parseAttributes($attributes).'></script>';
    }

    public static function stylesheet($src, array $attributes = array()){
        $attributes['type'] = 'text/css';
        $attributes['href'] = strstr($src, '://') ? $src : SITE_URL . $src;
        $attributes['rel'] = 'stylesheet';

        return '<link'.self::parseAttributes($attributes).'/>';
    }

    public static function a($url, $text = null, $target = '_self', array $attributes = array()){
        $attributes['href'] = strpos($url, '://') === false ? SITE_URL.$url : $url;
        $attributes['target'] = $target;

        if($text === null){
            $text = $attributes['href'];
        }

        return '<a'.self::parseAttributes($attributes).'>'.$text.'</a>';
    }

    public static function form($method = 'GET', $action = '', array $attributes = array(), $csrfToken = null, $content = null){
        if($csrfToken === null){
            $csrfToken = \Input::generateCSRFtoken();
        }

        $attributes['method'] = $method;
        $attributes['action'] = $action;

        return '<form'.self::parseAttributes($attributes).'>'.
                '<input type="hidden" name="csrf-token" value="'.$csrfToken.'"/>'.
                $content.
               '</form>';
    }

    public static function input($name, $type, $value = null, array $attributes = array(), $div = true){
        $attributes['name'] = $name;

        if($name === null){
            $div = '';
        }else{
            if($value === null && $type != 'password'){
                $value = \Input::data($name);
            }

            if($div){
                $div = '<div class="'.$name.' Field-Error">'.\Input::getValidationErrors($name).'</div>';
            }
        }

        switch($type){
            case 'textarea':
                $input = '<textarea'.self::parseAttributes($attributes).'>'.$value.'</textarea>';
                break;

            case 'select':
                if(is_array($value)){
                    $options = '';
                    foreach($value as $key => $option){
                        $options .= '<option value="'.$key.'">'.$option.'</option>';
                    }
                    $input = '<select'.self::parseAttributes($attributes).'><option>'.$options.'</option></select>';
                }
                break;

            default:
                $attributes['type'] = $type;
                $attributes['value'] = $value;
                $input = '<input'.self::parseAttributes($attributes).'/>';
        }

        return $div.$input;
    }

    private static function parseAttributes($attributes){
        foreach($attributes as $attribute => $value){
            $attributes[$attribute] = ' '.$attribute . '="'.$value.'"';
        }

        return join('', $attributes);
    }

    public static function sanitize($string, $flag = ENT_QUOTES, $encoding = 'UTF-8'){
        return htmlspecialchars($string, $flag, $encoding);
    }
}

A few details - the __callStatic function handles the majority of tags, and currently does not support self-closing tags. The whole class is currently a work in progress and I'd be really thankful if you share any bugs or improvements you see. I would like to also mention that if anyone finds this class useful they can use it freely (not that I can actually put any restrictions on that but just thought it's good to know).

Also about the sanitize function, I know it's not good to create function wrappers that do nothing else than call another function with the same parameters, but that's just something I foresaw. Maybe in the future I will need to put some custom sanitizing technique or something, then I'll have all of my projects automatically updated, as opposed to having to start implementing it everywhere.

\$\endgroup\$
  • 1
    \$\begingroup\$ Forgive me but: What's the point? PHP can be embedded in HTML easily (simply close ?>, write the markup and reopen <?php). Why add the overhead of static method calls and a pure evil magic __callStatic? Your api doesn't allow me to turn back to a node, change some attributes or add children to it. In the end, I'm still stuck writing rather old-school procedural styled code (with an OO front) \$\endgroup\$ – Elias Van Ootegem Oct 10 '14 at 15:02
  • \$\begingroup\$ Just for the record it already exists out there github.com/naomik/htmlgen and you should remember the DRY concept \$\endgroup\$ – elibyy Oct 10 '14 at 15:09
  • \$\begingroup\$ @elibyy DRY concept? And IMHO that object you referenced is extremely ugly and closures, really? \$\endgroup\$ – php_nub_qq Oct 10 '14 at 17:19
  • \$\begingroup\$ @EliasVanOotegem the point is that you need to write very less, as well as you can use php variables in elements' content without having to open and close php tags every time which I find to be a great pain for coding and reading, literally. Also I have automated error reporting on form submission and auto field refill in case of such, to do that with markup it is a bit more complicated and certainly not good looking. \$\endgroup\$ – php_nub_qq Oct 10 '14 at 17:22
  • \$\begingroup\$ Just saying DRY = don't repeat yourself. I'm just saying also. That you should do something more general then tag specific. Also that what I've found by quick googling \$\endgroup\$ – elibyy Oct 10 '14 at 17:22
9
\$\begingroup\$

The general idea

is it good or bad?

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

Advantages:

  1. 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).
  2. Fast to Use: it might be faster to use than writing HTML yourself.
  3. Minify: it can produce already minified HTML.
  4. 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:

  1. Speed: it will always be slower than native HTML.
  2. 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.
  3. 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 }).
\$\endgroup\$
  • \$\begingroup\$ Basically the class started as a form generator, then I renamed it to HTML and started creating these methods 1 by 1 in time. Up until today it hit me that I can use only php to create markup and the main idea that drove me was that I can use values from PHP variables without having to use the open and close php tags <?=$var?> which may not seem like much but it really makes a difference for me. Thank you for the nice review, I guess I will benchmark it and decide if the slowdown is significant enough to make me use markup instead. \$\endgroup\$ – php_nub_qq Oct 10 '14 at 17:31
  • \$\begingroup\$ @php_nub_qq I agree, PHP tags are quite distracting. If it's just about them, you might want to look at a template engine, such as twig, which I think looks a lot nicer than HTML with PHP mixed in. \$\endgroup\$ – tim Oct 10 '14 at 17:42
  • 1
    \$\begingroup\$ One potential advantage I could see to using a wrapper class versus raw HTML is that a wrapper class can take care of escapes and sanitization in a way that might be more difficult using raw HTML. If everything has to go through the wrapper, then it's possible to ensure that no unescaped greater-than signs can be created except by the wrapper itself. That would seem more difficult if code is expected to build things from HTML tags that would of course include such characters. \$\endgroup\$ – supercat Oct 10 '14 at 18:01
  • 1
    \$\begingroup\$ @php_nub_qq check my links. Generally, yes, htmlspecialchars prevents most cases of xss. But there are corner cases where it might not be enough (mainly when placing user input inside HTML attributes (as key or as value)). If you want to know more about it, this XSS Prevention Cheat Sheet is interesting. \$\endgroup\$ – tim Oct 10 '14 at 18:41
  • 1
    \$\begingroup\$ @tim I just finished benchmarking and I thought I should let you know what the results are just in case you are curious. With unrealistically small markup, like the one in the example, markup appears to be 6% faster than nested functions, however with slightly larger ( but still small ) response - 1024 bytes in size - the times are almost identical and by increasing the response size markup starts getting a little slower. I suppose that is due to unminified content but anyway the php way doesn't seem to be much slower if at all. \$\endgroup\$ – php_nub_qq Oct 10 '14 at 21:30
5
\$\begingroup\$

Even though the answer was already accepted, I want to argue a bit differently.

So practically, with your current approach, I think the answer is no, not really, unless you can cut down the number of tokens. Though if you can find a way to have just html(body(...)), then yes.

First of all, this is known as an embedded DSL since you're using language constructs to mimic HTML.

Now I don't think correctness is an issue here (unless you're very dilligent), since you won't be able to encode all the rules of HTML (which version even), not to mention CSS and other standards. And even if you did, you'd have to follow new developments and also fix compatibility issues, which you also won't have enough resources to do.

However, you gain an advantage here, which is more than a bit muddled, because you're already in a templating language anyway: You can use the regular tools of the language to create new kinds of abstractions instead of passing around strings. And you've already done that with the form and input functions. IMO that is a win.

Depending on what you want you could also generate a DOM instead of outputting strings, but using the same syntax, so you gain flexibility for different kinds of usages.

Lastly, you are able to stay in the syntax of your (primary) language. For some that is already enough of a reason to do this kind of thing. Usability-wise this could also be an improvement, since other people already have to know the programming language anyway (except if they just do design I guess, which is probably one of the main arguments against that), so staying within that framework can be less of an overhead than switching between two different systems, including interpolation rules, quoting, etc.

\$\endgroup\$
0
\$\begingroup\$

Sorry for poking and old dog but here is an idea, serialization.

This is not about right or wrong but my preferred solution is to, as far as possible, separate data from logic. The complex solution would be to create a class library for a HTML Document Object Model.

But on the other end of the spectrum is the simple solution with regular arrays and a (slightly evil) recursive function. Let's define keys as tags and values as content. An example of some data:

$data = array(
    'doctype',
    'html' => array(
        'head' => array(
            'meta charset="utf8"' => NULL,
            'title' => 'The page title',
        ),
        'body' => array(
            'div#header' => array('h1' => 'Page title'),
            'div#content' => 'Some content'
        )
    )
);

The generalized (but evil) recursive function where most of the magic happends:

function atrans($key, $value, &$output, $keyfilter=NULL, $valfilter=NULL, $keyendfilter=NULL, $pretty=FALSE){
    static $nest = 0;
    $nest++;
    $ind = $pretty ? str_repeat(" ", $nest) : '';
    if($key == NULL){
        if(is_array($value)){
            foreach($value as $key => $val){
                atrans($key, $val, $output, $keyfilter, $valfilter, $keyendfilter, $pretty);
            }
        }else{
            $output[] = $ind . (is_callable($valfilter) ? $valfilter((string)$value) : $value);
        }
    }elseif(is_integer($key)){
        if(is_array($value)){
            atrans(NULL, $value, $output, $keyfilter, $valfilter, $keyendfilter, $pretty);
        }else{
            $output[] = $ind . (is_callable($valfilter) ? $valfilter((string)$value) : $value);
        }
    }else{
        $keystart = is_callable($keyfilter) ? $keyfilter( $key ) : $key;
        $keyend = is_callable($keyendfilter) ? $keyendfilter( $key ) : '';

        if(is_array($value)){
            $output[] = $ind . $keystart;
            atrans(NULL, $value, $output, $keyfilter, $valfilter, $keyendfilter, $pretty);
            if(!empty($keyend)) $output[] = $ind . $keyend;

        }elseif($value !== NULL){
            $valstr = is_callable($valfilter) ? ($valfilter)((string)$value) : $value;
            $output[] = $ind . $keystart . $valstr . $keyend;
        }else{
            $output[] = $ind . $keystart;
            // $value is NULL, $endfilter will not run
        }
    }
    $nest--;
}

And then a specialization for HTML output:

// HTML specialization of the above
function array_to_html($arr, $pretty=FALSE){
    $openTag = function($key){
        $key = preg_replace('/#([a-z][a-z0-9\\-]*)$/i', ' id="$1"', $key);
        $key = preg_replace('/\\.([a-z][a-z0-9\\-]*)$/i', ' class="$1"', $key);
        // basic validation of attribute syntax
        if(preg_match('/^(\!?[a-z][a-z0-9\\-]*)(\\s[a-z][a-z0-9\\-\\.@]*(=[a-z0-9\\-\\.]+|=\'[^\']*?\'|="[^"]*?"))*$/i', $key) !== 1)
            echo 'TAG ERROR: ' . $key . '<br>';
        return "<$key>";
    };
    $valueFilter = function($value){
        $expand = array('doctype' => '<!DOCTYPE html>');
        $value = isset($expand[$value]) ? $expand[$value] : $value;
        if(mb_substr($value, 0, 1) == '<'){ // assume HTML, just print
            return (string)$value;
        }else{ // escape all other content
            return htmlspecialchars((string)$value, ENT_QUOTES, 'UTF-8', FALSE);
        }
    };
    $closeTag = function($key){
        $matches = NULL; preg_match('/^[^\\s\\.#]+/', $key, $matches);
        $close = $matches[0];
        return "</$close>";
    };

    $output = array();
    atrans(NULL, $arr, $output, $openTag, $valueFilter, $closeTag, $pretty);

    return join(($pretty ? "\n":""), $output);
}

// get output "pretty-printed"
$output = array_to_html($data, TRUE);

Disclaimer for the copy-pasters out there. The regular expressions in this code are incorrect and/or incomplete. They only serve to illustrate what you can do in a few lines.

Cons: Very general construct with no simple way to define what is what. Recursive functions are evil!

Pros: Easy to serialize, compare, process etc. Easy to add validation to suit your needs.

Expected output:

<!DOCTYPE html>
  <html>
    <head>
      <meta charset="utf8">
      <title>The page title</title>
    </head>
    <body>
      <div id="header">
        <h1>Page title</h1>
      </div>
      <div id="content">Some content</div>
    </body>
  </html>
\$\endgroup\$
  • 1
    \$\begingroup\$ Welcome to Code Review! You have presented an alternative solution, but haven't reviewed the code. Please edit to show what aspects of the question code prompted you to write this version, and in what ways it's an improvement over the original. It may be worth (re-)reading How to Answer. \$\endgroup\$ – Toby Speight Oct 4 at 10:56
  • \$\begingroup\$ Funny enough, 5 years later I've created what you just suggested. Except we live in a world ran by JavaScript nowadays but nevertheless here it is in case you are curious. \$\endgroup\$ – php_nub_qq Oct 4 at 11:40
  • \$\begingroup\$ Thanks for feedback, I felt that most of my opinions were covered by the other answers already. I just wanted to provide perspective on why the original solution might be lacking in some areas for the benefit of future visitors. \$\endgroup\$ – Daniklad Oct 5 at 22:19
  • \$\begingroup\$ Future visitors should avoid doing anything mentioned here whatsoever. Generating HTML is always going to be superior to serving it except maybe for SEO but complex applications don't need SEO anyway. Eventually crawlers will learn to generate it too, if they haven't yet. \$\endgroup\$ – php_nub_qq Oct 10 at 8:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.