3
\$\begingroup\$

I have the following script that reads JSONified data from within a DIV, generates HTML with values using JavaScript, then sends them to another DIV.

What I'd like to improve is to remove HTML from JavaScript. Namely, not have HTML inside the JavaScript. I am not 100% on how to do this. I want "minimal work" but "most separation" of HTML and JS.

var selection = eval("(" + document.getElementById("result").innerHTML + ")");

/*
 * Relevant Example only:
 * selection[index].couplings = "[{"price":13.00,"description":"Couplings"}]";
 */

/**
 * Show Couplings
 */
if (selection[index].couplings != null)
{
    for (i = 0; i < selection[index].couplings.length; i++) {
        result += '<TR>';
        result += '<TD>' + selection[index].couplings[i].description + '</TD>';
        result += '<TD style="text-align: right;">' + moneyFormat(selection[index].couplings[i].price) + '</TD>';
        result += '</TR>';
    }
}

document.getElementById('couplings').innerHTML = result;


/*****************/
/* Number Format */
/*****************/
function moneyFormat(num) {
    return '$' + parseFloat(num).toFixed(2).replace(/(\d)(?=(\d{3})+\.)/g, "$1,").toString();
}
\$\endgroup\$

1 Answer 1

5
\$\begingroup\$
var selection = eval("(" + document.getElementById("result").innerHTML + ")");

I'm assuming this is the part that reads the JSON? There's JSON.parse for that. Feed it a JSON string and it should give you the equivalent object. No need to use eval.

/*****************/
/* Number Format */
/*****************/
function moneyFormat(num) {
    return '$' + parseFloat(num).toFixed(2).replace(/(\d)(?=(\d{3})+\.)/g, "$1,").toString();
}

I would suggest putting functions and any other inert operations higher up in the code. It's mostly for readability, knowing what exists before trying to use them.

Another is that parseFloat may return NaN. You may want to guard by checking against isNaN before handing it over to other operations.

Lastly, concatenating a string to a number (i.e.: 'foo' + 2) will automatically coerce the number to a string. Therefore you don't need the trailing toString().

if (selection[index].couplings != null)
{
    for (i = 0; i < selection[index].couplings.length; i++) {
        result += '<TR>';
        result += '<TD>' + selection[index].couplings[i].description + '</TD>';
        result += '<TD style="text-align: right;">' + moneyFormat(selection[index].couplings[i].price) + '</TD>';
        result += '</TR>';
    }
}

There's 2 ways to avoid writing markup in JS. One would be to construct the elements using document.createElement or by using a template library and construct the markup elsewhere.

A third option, if you have ES6 at your fingertips is to use template strings. This allows you to write multi-line strings in JS. And guess what? We can do something like this:

result += `
  <tr>
    <td>${selection[index].couplings[i].description}</td>
    <td>${moneyFormat(selection[index].couplings[i].price)}</td>
  <tr>
`;
\$\endgroup\$
1
  • \$\begingroup\$ thanks, as for the ES6, you suggest using HTML + JS variables inside HTML. So in a way just a more concise and convenient way to keep HTML with JS. Would it be correct to say that HTML and JS are okay to keep together just like it was in my Question? i.e. I don't have a good reason to go after separation of data & markup \$\endgroup\$
    – Dennis
    Commented Jun 20, 2016 at 15:40

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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