Simplifying Counter of Checked Checkboxes

Question

Is the following code a proper solution to my problem? I have a feeling it can be simplified quite a bit.

If one of the first four checkboxes is checked the sum is 1; if two 1.5; if three 2; if four 2.5. If the bar5 is checked then it's 1 + calculated value of first four (if any is checked). Afterwards, multiply by 100.

function checkboxCount() {
	var form = document.forms["calculator"];
	var bar5 = form.elements["bar5"];
	var foo = document.querySelectorAll("input[name=foo]");
	count = 0;
	
	for (var i = 0; i < foo.length; i++) {
		if (foo[i] && foo[i].checked == true) {
			count++;
		}
	}
	if (count == 1 && bar5.checked == true) {
		count = 1;
	} else if (count == 1) {
		count = 1;
	}
	if (count == 2 && bar5.checked == true) {
		count = 2;
	} else if (count == 2) {
		count = 1.5;
	}
	if (count == 3 && bar5.checked == true) {
		count = 2.5;
	} else if (count == 3) {
		count = 2;
	}
	if (count == 4 && bar5.checked == true) {
		count = 3;
	} else if (count == 4) {
		count = 2.5;
	}
	if (count == 5 && bar5.checked == true) {
		count = 3.5;
	}
	return count;
}
function calculateTotalCost() {
	var totalCost = 0;
	var printCost = document.getElementById('calculatedCost');
	totalCost = 100 * checkboxCount();
	printCost.innerHTML = "$" + totalCost;
}

<form id="calculator">
<fieldset>
<label><input type="checkbox" name="foo" id="bar1" onclick="calculateTotalCost()">Bar 1</label>
<label><input type="checkbox" name="foo" id="bar2" onclick="calculateTotalCost()">Bar 2</label>
<label><input type="checkbox" name="foo" id="bar3" onclick="calculateTotalCost()">Bar 3</label>
<label><input type="checkbox" name="foo" id="bar4" onclick="calculateTotalCost()">Bar 4</label>
<label><input type="checkbox" name="foo" id="bar5" onclick="calculateTotalCost()">Bar 5</label>
</fieldset>
<span id="calculatedCost"></span>
</form>

Answer 1

1. The formula can be simplified:

   1. for the first four checkboxes, if any of the items are checked, multiply the number checked by 0.5 and add 0.5
   2. Then add 1 if the fifth checkbox is checked.
      
      

   So it might be simpler to just have a reference to the first for checkboxes and then the 5th checkbox. For the first for, one can use document.getElementsByName('foo') to get a NodeList, and then put those nodes in an array using Array.from() and use Array.slice() to get the first four checkboxes.

    var firstFourBars = Array.from(document.getElementsByName('foo')).slice(0, 4);
   

   _{For the record - I did consider using a CSS selector identical to that of the answer by @200_success (i.e. input[name=foo]:not(#bar5):checked) but because the NodeList returned by document.querySelectorAll() is not live, that DOM query would need to be run each time. See point #3 for more details.}

2. Event delegation can be used to avoid adding a click handler to each checkbox in the HTML. The code below utilizes document.addEventListener() to call the calculateTotalCost whenever a click occurs. Because of this change, that function receives a reference to the event as the first argument. This allows changes based on any click anywhere but conditional logic dependent on event.target could be added.

3. DOM references can be stored in variables at the start of the script, so as to avoid multiple lookups (e.g. var printCost)

4. One other simplification:

   var totalCost = 0;
   

    var printCost = document.getElementById('calculatedCost');
    totalCost = 100 * checkboxCount();
   

   There is little point in setting totalCost to 0 and then assigning it 100 * checkboxCount(). Just assign it the latter product:

    var totalCost = 100 * checkboxCount();
   

document.addEventListener('DOMContentLoaded', function() { //Wait for DOM to be ready
 //cache DOM references
 var printCost = document.getElementById('calculatedCost');
 var firstFourBars = Array.from(document.getElementsByName('foo')).slice(0, 4);
 var bar5 = document.forms["calculator"].elements["bar5"];
 document.body.addEventListener('click', calculateTotalCost);
 // helper function for filter
 function isChecked(checkbox) {
 return checkbox.checked;
 }
 function checkboxCount() {
 var count = firstFourBars.filter(isChecked).length;
 if (count) {
 count = count * 0.5 + 0.5;
 }
 if (bar5.checked) {
 count += 1;
 }
 return count;
 }
 function calculateTotalCost(event) {
 //could conditionally execute based on event.target
 var totalCost = 100 * checkboxCount();
 printCost.innerHTML = "$" + totalCost;
 }
});

<form id="calculator">
 <fieldset>
 <label><input type="checkbox" name="foo" id="bar1">Bar 1</label>
 <label><input type="checkbox" name="foo" id="bar2">Bar 2</label>
 <label><input type="checkbox" name="foo" id="bar3">Bar 3</label>
 <label><input type="checkbox" name="foo" id="bar4">Bar 4</label>
 <label><input type="checkbox" name="foo" id="bar5">Bar 5</label>
 </fieldset>
 <span id="calculatedCost"></span>
</form>

Edit

I forgot to mention that in the modified code above, it waits for the DOM to be ready before before querying it (notice the first line document.addEventListener('DOMContentLoaded', function() { //Wait for DOM to be ready). Not only does it prevent attempts to access DOM elements before they exist, but the variables are scoped to the callback function instead of the global namespace.

• sineemore: It might make sense to call the function calculateTotalCost directly for the first time.
• Sᴀᴍ Onᴇᴌᴀ: Yeah - maybe... though the OPs code doesn't function that way...
• Tzar: Hey @SamOnela, I just noticed a bug. If you click on Bar 5 first, the value remains 0ドル instead of 100ドル. Could you please help me? I would greatly appreciate it.
• Sᴀᴍ Onᴇᴌᴀ: Ah yes- I have updated the code in my answer to handle that.
• Tzar: It works beautifully now. Thank you so much!

Answer 2

You aren't taking advantage of the capabilities of document.querySelectorAll(). Furthermore, checkboxCount() would have much simpler logic if you interpreted the requirements more literally: you should count the first four checkboxes separately from the fifth. Finally, I would complain that checkboxCount() is inappropriately named, since it has logic that is much more complicated than counting.

The querySelectorAll() function takes a CSS selector. Not only can the selector specify input[name=foo], but you can also exclude a specific element using :not() and filter by the :checked status.

Instead of if-else cases to determine the "sum", I'd use an array lookup.

Instead of writing an onclick handler for each input element, it would be better practice to eliminate all traces of JavaScript from the HTML, and attach the handlers using JavaScript.

function count(selector) {
 return document.querySelectorAll(selector).length;
}
function calculateTotalCost() {
 var totalCost = 100 * (
 [0, 1, 1.5, 2, 2.5][count("input[name=foo]:not(#bar5):checked")] +
 count("#bar5:checked")
 );
 document.getElementById('calculatedCost').innerHTML = "$" + totalCost;
}
document.querySelectorAll("input[name=foo]").forEach(function(el) {
 el.addEventListener('click', calculateTotalCost);
});

<form id="calculator">
<fieldset>
<label><input type="checkbox" name="foo" id="bar1">Bar 1</label>
<label><input type="checkbox" name="foo" id="bar2">Bar 2</label>
<label><input type="checkbox" name="foo" id="bar3">Bar 3</label>
<label><input type="checkbox" name="foo" id="bar4">Bar 4</label>
<label><input type="checkbox" name="foo" id="bar5">Bar 5</label>
</fieldset>
<span id="calculatedCost"></span>
</form>

• Tzar: Thank you for such a comprehensive answer. I really appreciate it. Although your code is more concise, Sam’s solution fits better in my form, which is much larger than the code I posted.
• 200_success: It's not a good idea to post Code Review questions that are simplified versions of your real code, because then we can't give you the most appropriate advice.
• Tzar: I did not know that. It makes sense. Thank you.
• Tzar: I posted my full code if you’d like to take a look: Simplifying and Improving Form Calculator