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.

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>