# Submitting form jQuery checkboxlist

In my requestForm submit function I have this logic that checks three checkboxes and then displays the label with an error message. Is there better logic? I would be happy for any kind of feedback since this is quite critical a part of my application.

$("#requestForm").submit(function (e) { e.stopPropagation(); e.preventDefault(); var checked_sourcecheckboxes =$("#sources input[type=checkbox]:checked");
if (checked_sourcecheckboxes.length == 0) {
$("#lblSourcesError").show(); return false; } else {$("#lblSourcesError").hide();
}
var checked_academicformatcheckboxes = $("#academicFormatsRow input[type=checkbox]:checked"); if (checked_academicformatcheckboxes.length == 0) {$("#lblacademicFormatError").show();
return false;
}
else {
$("#lblacademicFormatError").hide(); } var checked_academicmediaformatcheckboxes =$("#academicMediaFormatRow input[type=checkbox]:checked");
$("#lblacademicmediaFormatError").show(); return false; } else {$("#lblacademicmediaFormatError").hide();
}
})

• what about the return false; logic? that will stop the loop – Jefferson Feb 9 at 16:08
• You also should post the HTML. Because I would suggest some change... It seems you only use id and you could have a way simpler logic if you'd use classes on the checkboxes and error messages. – Louys Patrice Bessette Feb 9 at 18:39

## Main Question

Is there better logic?

### Repeated code

The first thing that I see with this code, which perhaps you see also, is that it is quite repetitive - select a list of checkboxes to see if any are checked - if none are checked then show an error message and return, otherwise hide any affiliated error message. This violates the Don't Repeat Yourself principle. It can be remedied by abstracting the common logic - e.g. putting the the selectors into a mapping:

var SELECTOR_ERROR_MAPPING = {
"#sources input[type=checkbox]:checked": "#lblSourcesError",
//...more entries
};



Then use a loop to iterate over the mapping, checking each selector - e.g. using Array.every():

return Object.keys(SELECTOR_ERROR_MAPPING).every(function(selector) {
var errorSelector = SELECTOR_ERROR_MAPPING[selector];

var showError = !$(selector).length;$(errorSelector).toggle(showError, errorSelector);
return !showError;
});


Those last three lines of the inner function could be placed into a separate function if desired, which would allow for better modularization/atomization and testing.

### demo

var SELECTOR_ERROR_MAPPING = {
"#sources input[type=checkbox]:checked": "#lblSourcesError",
//...more entries
};
$("#requestForm").submit(function(e) { e.stopPropagation(); e.preventDefault(); var allValid = Object.keys(SELECTOR_ERROR_MAPPING).every(function(selector) { var errorSelector = SELECTOR_ERROR_MAPPING[selector]; var showError = !$(selector).length;

### demo

const SELECTOR_ERROR_MAPPING = {
"#sources input[type=checkbox]:checked": "#lblSourcesError",
//...more entries
};
e.stopPropagation();
e.preventDefault();

for (const [selector, errorSelector] of Object.entries(SELECTOR_ERROR_MAPPING)) {
const showError = !document.querySelectorAll(selector).length;
document.querySelector(errorSelector).classList.toggle('hidden', !showError);
if (showError) {
return false;
}
}
});
.error {
color: red;
}
.hidden {
display: none;
}
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<form id="requestForm">
<div id="sources">
<h3> Source </h3>
<label>Audi
<input type="checkbox" name="source" value="Audi" />
</label>
<label>BMW
<input type="checkbox" name="source" value="BMW" />
</label>
<div class="error hidden" id="lblSourcesError">Please select a source</div>
</div>
<h3> Formats </h3>
<label>College
<input type="checkbox" name="source" value="college" />
</label>
<label>University
<input type="checkbox" name="source" value="university" />
</label>

</div>
</form>