sKunKbad Posted June 30, 2010 Share Posted June 30, 2010 I tried to take this script and make an array of form element names, and then use them in a for in statement, but I wasn't doing it right. This script works. It calculates the totals of some radio buttons, and then sends that total along with a posted form, where it is processed by php. I just want to simplify it, and am apparently a little rusty (or I suck) at javascript. function calculate_totals() { $section1_total = 0; for (i = 0; i < 3; i++) { if (document.form_8.I_something[i].checked == true) { $section1_total += parseInt(document.form_8.I_something[i].value); } } for (i = 0; i < 3; i++) { if (document.form_8.I_another[i].checked == true) { $section1_total += parseInt(document.form_8.I_another[i].value); } } for (i = 0; i < 3; i++) { if (document.form_8.I_whatever[i].checked == true) { $section1_total += parseInt(document.form_8.I_whatever[i].value); } } for (i = 0; i < 3; i++) { if (document.form_8.I_thing[i].checked == true) { $section1_total += parseInt(document.form_8.I_thing[i].value); } } document.form_8.section1_total.value = $section1_total; return; } So you can see where there is some obvious repetition here. The problem was that when I was trying to use the name of the radio button set inside of the "document.form_8.xxxxxx.value", it wouldn't work. Is this possible? Quote Link to comment Share on other sites More sharing options...
sKunKbad Posted June 30, 2010 Author Share Posted June 30, 2010 Here is what doesn't work: <?php if( isset( $_POST ) && ! empty ( $_POST ) ) { echo "<pre>"; print_r ( $_POST ); echo "</pre>"; } ?> <script type="text/javascript"> function calculate_totals() { $section1_total = 0; var names = ["I_something","I_another","I_whatever","I_thing"]; for ( var x in names ) { // each radio button set has three options, but none are required for (i = 0; i < 3; i++) { var form_element = names[x] + [i]; if (document.the_form[form_element].checked == true) { $section1_total += parseInt(document.the_form[form_element].value); } } } document.the_form.section1_total.value = $section1_total; return; } </script> <form name="the_form" action="" method="post" onSubmit="calculate_totals();"> <p>Something</p> 1<input type="radio" name="I_something" value="1" /><br /> 3<input type="radio" name="I_something" value="3" /><br /> 5<input type="radio" name="I_something" value="5" /> <p>Another</p> 1<input type="radio" name="I_another" value="1" /><br /> 3<input type="radio" name="I_another" value="3" /><br /> 5<input type="radio" name="I_another" value="5" /> <p>Whatever</p> 1<input type="radio" name="I_whatever" value="1" /><br /> 3<input type="radio" name="I_whatever" value="3" /><br /> 5<input type="radio" name="I_whatever" value="5" /> <p>Thing</p> 1<input type="radio" name="I_thing" value="1" /><br /> 3<input type="radio" name="I_thing" value="3" /><br /> 5<input type="radio" name="I_thing" value="5" /> <p>Submit</p> <input type="hidden" name="section1_total" value="" /> <input type="submit" value="submit" /> </form> Quote Link to comment Share on other sites More sharing options...
sKunKbad Posted July 1, 2010 Author Share Posted July 1, 2010 This ends up working, but I don't know if it is the best way: <script type="text/javascript"> function calculate_totals() { $section1_total = 0; var names = ["I_something","I_another","I_whatever","I_thing"]; for ( var x in names ) { // each radio button set has three options, but none are required for (i = 0; i < 3; i++) { var element_checked = "document.the_form." + names[x] + "[" + i + "].checked"; if (eval(element_checked) == true) { var element_value = "document.the_form." + names[x] + "[" + i + "].value"; $section1_total += parseInt(eval(element_value)); } } } document.the_form.section1_total.value = $section1_total; return; } </script> Quote Link to comment Share on other sites More sharing options...
KevinM1 Posted July 1, 2010 Share Posted July 1, 2010 You're on the right track, but your use of eval() should be a warning sign. As a rule of thumb, if you use eval(), you're doing it wrong. Basically, you need to loop through each set of radio buttons, and figure out which one is checked. A simple for-loop is all you need. var total = 0; var oAnother = document.forms['the_form'].elements['l_another']; for(var i = 0; i < oAnother.length; ++i) { if (oAnother[i].checked) { total += parseInt(oAnother[i].value); } } Repeat for the others. Quote Link to comment Share on other sites More sharing options...
sKunKbad Posted July 1, 2010 Author Share Posted July 1, 2010 Repeat for the others. The problem is, there are many others in the actual implementation of the code, and I was hoping to make the code as small as possible. I'm not super good at javascript (obviously), so I wasn't aware that eval was evil. Quote Link to comment Share on other sites More sharing options...
KevinM1 Posted July 1, 2010 Share Posted July 1, 2010 Repeat for the others. The problem is, there are many others in the actual implementation of the code, and I was hoping to make the code as small as possible. I'm not super good at javascript (obviously), so I wasn't aware that eval was evil. You can still loop through them all without hard coding the radio button names. Use document.getElementsByTagName('input') to get an array of all form inputs. Your idea of storing a white list of radio button names is a good idea. You can do something like: var allowed = new Array(); // populate it with the names of your radio buttons var radios = new Array(); // this is where all the valid buttons go for(var i = 0; i < inputs.length; ++i) { for(var j = 0; j < allowed.length; ++j) { if (inputs[i].name == allowed[j]) { radios.push(inputs[i]); } } } From there a simple for-loop over the radios array should do the trick. As a general tip, don't be afraid to use multiple loops in succession to filter and process elements. It may be more typing, but it's more efficient than nested loops, and has the benefit of clarity and readablity. Quote Link to comment Share on other sites More sharing options...
sKunKbad Posted July 1, 2010 Author Share Posted July 1, 2010 So then something like this is more kosher?: function do_calc() { var inputs = document.getElementsByTagName('input'); var allowed = new Array("I_something","I_another","I_whatever","I_thing"); var radios = new Array(); var section1_total = 0; for(var i = 0; i < inputs.length; ++i) { for(var j = 0; j < allowed.length; ++j) { if (inputs[i].name == allowed[j]) { radios.push(inputs[i]); } } } for(var x in radios) { if(radios[x].checked == true) { section1_total += parseInt(radios[x].value); } } document.the_form.section1_total.value = section1_total; } It works too. Quote Link to comment Share on other sites More sharing options...
KevinM1 Posted July 1, 2010 Share Posted July 1, 2010 Much better :thumbs up: Quote Link to comment Share on other sites More sharing options...
sKunKbad Posted July 1, 2010 Author Share Posted July 1, 2010 Thanks for your help. I wish I knew more about javascript, but I don't use it enough. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.