patternjavascriptMinor
Displaying user selection in the browser
Viewed 0 times
selectiontheuserbrowserdisplaying
Problem
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.
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 ';
result += '' + selection[index].couplings[i].description + '';
result += '' + moneyFormat(selection[index].couplings[i].price) + '';
result += '';
}
}
document.getElementById('couplings').innerHTML = result;
/*****************/
/* Number Format */
/*****************/
function moneyFormat(num) {
return ' + parseFloat(num).toFixed(2).replace(/(\d)(?=(\d{3})+\.)/g, "$1,").toString();
}Solution
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 '
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 ';
result += '' + selection[index].couplings[i].description + '';
result += '' + moneyFormat(selection[index].couplings[i].price) + '';
result += '';
}
}
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 += `
${selection[index].couplings[i].description}
${moneyFormat(selection[index].couplings[i].price)}
`;
+ 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().%%CODEBLOCK_2%%
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:
%%CODEBLOCK_3%%
Code Snippets
var selection = eval("(" + document.getElementById("result").innerHTML + ")");/*****************/
/* Number Format */
/*****************/
function moneyFormat(num) {
return '$' + parseFloat(num).toFixed(2).replace(/(\d)(?=(\d{3})+\.)/g, "$1,").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>';
}
}result += `
<tr>
<td>${selection[index].couplings[i].description}</td>
<td>${moneyFormat(selection[index].couplings[i].price)}</td>
<tr>
`;Context
StackExchange Code Review Q#132326, answer score: 5
Revisions (0)
No revisions yet.