2
\$\begingroup\$

Currently when the user selects some tags in my app I generate them in a function, like this:

// Show tags that are currently used in search
var htmlNodes = '<ul class="tags">';

// If no pinned nodes, reset not found nodes
if (graphFactory.existPinnedNodes() == false) {
  graphFactory.updateNotFoundNodes();
}

// Add not found nodes as tags (gray)
for (let x = 0; x < graphFactory.getNotFoundNodes().length; x++) {
  htmlNodes = htmlNodes + '<li><a class="notfound" href="#">' + graphFactory.getNotFoundNodes()[x] + '</a></li>';
}

// Add pinned nodes as tags (blue)
for (let q = 0; q < graphFactory.getPinnedNodes().length; q++) {
  htmlNodes = htmlNodes + '<li><a href="#">' + graphFactory.getPinnedNodes()[q] + '</a></li>';
}

htmlNodes = htmlNodes + '<div class="tagsend"></div></ul>';
$('#pinnednodeslist').html(htmlNodes);

Basically I create a <ul> of class tags, add some li elements with a specific tag each, and then render it into the #pinnednodeslist element.

As you can see, the number of <li> elements might change, depending on the number of getPinnedNodes or getNotFoundNodes, so I wonder if I have to reconstruct this html every time or if there is a more efficient way to do that...

Maybe a better way is that I don't have to mix so much HTML with my code?

For instance, I was thinking to leave all the elements there and just make them invisible, simply adding a class to show them and the html content to display within... But maybe you know of some other better way to do that?

\$\endgroup\$
1
\$\begingroup\$

First off: It's invalid HTML to have a div directly inside an ul element. Any special reason you need the <div class="tagsend"></div>? Modern CSS techniques usually don't need additional elements like that.


You shouldn't repeatedly call the getter methods in a loop like that. If they do more work than just returning an array, then it's wasted performance.

let notFoundNodes = graphFactory.getNotFoundNodes();
for (let x = 0; x < notFoundNodes.length; x++) {
  htmlNodes = htmlNodes + '<li><a class="notfound" href="#">' + notFoundNodes[x] + '</a></li>';
}

Or, considering you are using let which is part of ECMAScript 2015, you could alternatively use a for ... of loop:

for (let notFoundNode of graphFactory.getNotFoundNodes()) {
  htmlNodes = htmlNodes + '<li><a class="notfound" href="#">' + notFoundNode + '</a></li>';
}

Concatenating strings in loops are also quite a wasteful operation, because they cannot be optimized by the runtime environment. A better way is to collect the strings in an array, and then joining them together at the end:

var htmlNodes = ['<ul class="tags">'];

// ...

for (let notFoundNode of graphFactory.getNotFoundNodes()) {
  htmlNodes.push('<li><a class="notfound" href="#">' + notFoundNode + '</a></li>');
}

// ...

htmlNodes.push('</ul>');
$('#pinnednodeslist').html(htmlNode.join(''));

In order to avoid mixing the HTML you could use a template engine. There are literally dozens out there, with many pros and cons, which can make the choice a bit overwhelming. For an example I'll use Handlebars.js:

The template is placed inside a script element somewhere in your HTML:

<script id="tags-template" type="text/x-handlebars-template">
  <ul class="tags">
    {{#each notFoundNodes}}
      <li><a class="notfound" href="#">{{this}}</a></li>
    {{/each}}
    {{#each pinnedNodes}}
      <li><a href="#">{{this}}</a></li>
    {{/each}}
  </ul>
</script>

Once during page initialization you compile the template:

var source = document.getElementById("tags-template").innerHTML;
var tagsTemplate = Handlebars.compile(source); 

And then later you can repeatedly use the compiled template to render the data:

var context = {
   notFoundNodes: graphFactory.getNotFoundNodes(),
   pinnedNodes: graphFactory.getPinnedNodes()
};
var html = tagsTemplate(context);
$('#pinnednodeslist').html(html);

EDIT: BTW, nothing in this (or your original code) really requires jQuery. The last line could simply replaced with:

document.getElementById('pinnednodeslist').innerHtml = html;

The next step could be to use more advanced library such as React, Vue.js or Angular. These are frameworks that not only render the data with a template engine, but also can (among other things) react to changes in the data and re-render automatically when even needed. This however requires learning a bit of a different way of programming.


Finally, if the data displayed is coming from the server (via AJAX for example), then you could consider having the server render the HTML instead of sending the raw data.

\$\endgroup\$

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.