3
\$\begingroup\$

I'm trying to create a function for a dynamic dropdown. I have done it this way and it works, but I want to make my code cleaner.

 function renderRowCounter() {

    var rowCount = [25, 50, 100];
    var first = rowCount[0];
    var items = '';

    $.each(rowCount, function(i,v) {
        items += "<li><a href=\"#\">" + v + "</a></li>";
    });

    return  "<div class=\"btn-group btn-group-sm\">" +
            "<button type=\"button\" data-toggle=\"dropdown\" class=\"btn btn-default dropdown-toggle\">" + first + " <span class=\"caret\"></span></button>" +
            "<ul class=\"dropdown-menu\">" + items +
            "</ul>" +
            "</div> ";
}
\$\endgroup\$
1
\$\begingroup\$

Looks pretty clean for me.

2 things:

  1. I'm not an English expert, but check the name of the function and the action. I understand "render" as to "display on the screen", or "write on the screen," but the function doesn't really write to the screen. It returns some template to be injected later.

  2. Instead of using a concatenated string, you can use a template engine to avoid concatenating strings. It separates the logic from the template and makes both more maintainable. The most popular ones are Mustache, Handlebars, and the Backbone engine.

\$\endgroup\$
  • \$\begingroup\$ yeah, I would use 'generate' instead of 'render'. \$\endgroup\$ – dwjohnston Mar 24 '16 at 4:06
1
\$\begingroup\$

I agree with @vmariano, this is clean, but a template engine will help with the concatenations.

These anonymous function parameters here are not very descriptive:

$.each(rowCount, function(i,v) {
    items += "<li><a href=\"#\">" + v + "</a></li>";
});

Single-letter variable names are not recommended in general, so this is better:

$.each(rowCount, function(index, value) {
    items += "<li><a href=\"#\">" + value + "</a></li>";
});

When you need to embed double-quotes in a string, consider enclosing the string in single-quotes instead, for example:

    items += '<li><a href="#">' + value + "</a></li>";

This will be even more useful in the longer string in your return statement.

\$\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.