patternjavascriptMinor
Set of radio buttons
Viewed 0 times
buttonsradioset
Problem
I wrote a quick function to create a set of radio buttons, three per table row. I would like some feedback about how I can shorten this bit of code using algebraic functions, or different jQuery functionality.
var arr = [ "earth", "mars", "jupiter", "saturn", "venus", "argus", "pluto", "janus", "canary", "orange", "butter pineapple", "eventual bliss"];
var obj = { one:"earth", two:"mars", three:"jupiter", four:"saturn", five:"venus" };
var start = ""; end = "";
var count = 0;
$("#radio_container").append("");
$.each(arr, function(i, val) {
if(count == 3)
{
start = "";
end = ""
count= 0;
}
$("#radio_container").append(start);
$("#radio_container").append(""+val+"");
$("#radio_container").append(end);
count++;
start = "";
end = "";
});
$("#radio_container").append("");Solution
Here are some points to consider:
-
Your
-
As it stands now, your
-
Instead of manually keeping
-
jQuery's
-
Access to the DOM is not free. Every time you hit the DOM you incur some overhead. Try keeping DOM modification to a minimum.
-
Unlike others, when dealing with a simple HTML construct such as this, I prefer to build my fragment from a concatenated string since it's much faster.
With all that in mind, here's how I would do it:
See it here in action: http://jsfiddle.net/DWcSN/
The code above works, but the text is not associated with the radio button in any way.
To remedy that, consider wrapping labels around your radio buttons:
See it here in action: http://jsfiddle.net/DWcSN/1/ (clicking the text activates the corresponding button).
P.S.
-
Your
end variable is declared without a var, so that it's leaking into the global namespace. Don't do that.-
As it stands now, your
obj isn't used anywhere in the code. I'll assume it's not applicable here.-
Instead of manually keeping
count of your iteration, use the % (Modulus) operator to calculate your rows.-
jQuery's
append method doesn't deal with code fragments the way you seem to think it does. append("") actually appends a whole table, not just an opening tag (that's not even possible).-
Access to the DOM is not free. Every time you hit the DOM you incur some overhead. Try keeping DOM modification to a minimum.
-
Unlike others, when dealing with a simple HTML construct such as this, I prefer to build my fragment from a concatenated string since it's much faster.
With all that in mind, here's how I would do it:
var arr = ["earth", "mars", "jupiter", "saturn", "venus", "argus", "pluto", "janus", "canary", "orange", "butter pineapple", "eventual bliss"],
snippet = '';
$.each(arr, function(i, val) {
if (i % 3 == 0) snippet += '';
snippet += '' +
'' + val + '';
});
snippet += '';
$("#radio_container").append( snippet );
See it here in action: http://jsfiddle.net/DWcSN/
The code above works, but the text is not associated with the radio button in any way.
To remedy that, consider wrapping labels around your radio buttons:
snippet += '' +
'' +
val +
'';See it here in action: http://jsfiddle.net/DWcSN/1/ (clicking the text activates the corresponding button).
P.S.
style3 is not very descriptive. You should consider coming up with a more intuitive naming convention.Code Snippets
snippet += '<td><label class="style3">' +
'<input name="finalcategory" type="radio" value="' + val + '" />' +
val +
'</label></td>';Context
StackExchange Code Review Q#14497, answer score: 4
Revisions (0)
No revisions yet.