Toggling checkboxes based on certain conditions

Question

I have a bunch of checkboxes that need to be toggled based on certain conditions. My function is called by multiple buttons on an onclick event. The certain condition is expressed as a string (check_all, uncheck_all, check_all_virtual). The problem that I see is that if the function would be called as toggleCheckboxes('typo'), that the error wouldn't be so easy to see as there would be no error message. Such problems should probably yield more information. How could this be done better?

<div>
    <input name="site_ids[]" data-is-virtual="0" value="1" class="checkbox"/> SG3
    <input name="site_ids[]" data-is-virtual="1" value="2" class="checkbox"/> SG1
    <input name="site_ids[]" data-is-virtual="0" value="3" class="checkbox"/> SG2
</div>
<div>
    <button onclick="toggleCheckboxes('check_all')">Check All</button>
    <button onclick="toggleCheckboxes('uncheck_all')">Uncheck All</button>
    <button onclick="toggleCheckboxes('check_all_virtual')">Check All Virtual</button>
</div>

<script type="text/javascript">
var checkboxes = document.getElementsByClassName('checkbox');

function toggleCheckboxes(operation) {
    for (var i = 0, n = checkboxes.length; i < n; i++) {
        switch (operation) {
            case 'check_all':
                checkboxes[i].checked = true;
                break;
            case 'uncheck_all':
                checkboxes[i].checked = false;
                break;
            case 'check_all_virtual':
                if (checkboxes[i].getAttribute('data-is-virtual') == true) {
                    checkboxes[i].checked = true;
                }
                break;
        }
    }
}
</script>

The functionality is correct. Checking all virtual shouldn't uncheck non-virtual.

Answer 1

I may have misunderstood something but if you just want to be able to detect case where the input string is none of expected, you may do something like that:

default:
  console.log("This operation '"+operation+"' is not supported, it will be ignored");

at the end of your switch\case construction

Answer 2

Not sure what you want to achieve on this code, anyway I see the following improving points.

In this case, your code have what I think is a bug.

If you check all and then you check virual, I would expect that only the virtual checkbox will be checked, and other no.

In your code nothing will change.

Than you access this attribute directly from DOM getAttribute() call, instead you should access by dataset, as this kind of attributes are special in HTML5.

As the data-is-virtual value is a string you shouldn't compare with a boolean value, as you assign integers (0|1).

I suggest to change the code like so:

checkboxes[i].checked = (+checkboxes[i].dataset.isVirtual == 1);

The + force to convert the string to an integer value.

In this way only the data-is-virtual will be checked, and the others will be unchecked as expected.

But about this, I'm not sure what you want, I just say what I will expect to happen if I use this code.

The use of the switch helps the readability but if you want to cut your code, you could do like so:

if (operation == 'check_all_virtual') {
    checkboxes[i].checked = new Boolean(checkboxes[i].dataset.isVirtual);
} else {
    checkboxes[i].checked = (operation == 'check_all');
}

Another point of improvement is to use the arrays functions instead of a for loop:

function toggleCheckboxes(operation) {
    if (operation == 'check_all_virtual') {
        checkboxes.forEach(function (checkElement) {
            if (operation == 'check_all_virtual') {
               checkElement.checked = new Boolean(checkElement.dataset.isVirtual);
            } else {
               checkElement.checked = (operation == 'check_all');
            }
        });
    }
}