trjhtr

In jQuery, I have been reluctant to create a better form validation, and I feel the time has come for me to do so.

With that said, I have been using this particular type of form validation, and I would like to simplify it.

$('#saveSubmit').on('click', function()

 var criteria = 
 
 company: $('#company').val(),
 partnercode: $('#partnercode').val(),
 office: $('#office').val(),
 // few more
 

 // form validation starts here
 if(criteria.company == "")
 
 $('#compError').show();
 return false;
 
 if(criteria.partnercode == "")
 
 $('#compError').show();
 return false;
 
 if(criteria.office == "")
 
 $('#compError').show();
 return false;
 
 // few more if statements
 else
 
 // if the form parameters are good, send to processing script
 $.post('process/editUser.php', criteria:criteria, function(data)
 
 if(data.indexOf("Error") >= 0)
 
 $('#errorModal').modal('show');
 $('#message').text(data);
 return false;
 
 else
 
 $('#successModal').modal('show');
 $('#messageSuccess').text(data); 
 $('#successModal').on('hidden.bs.modal', function()
 
 $('#successModal').modal('hide');
 location.reload();
 );
 
 );
 
);

All of this works like how it should, but I want to make it less verbose, thus the need for a function.

I want to replace that huge if/else statement within that click event with a function.

You might break apart the code in separate functions, and create a function that opens and closes a modal with a custom text. I didn't check this code, hopefully you get some ideas for improvement!

$('#saveSubmit').on('click',()=>validateForm())

function validateForm()

 var criteria = 
 
 company: $('#company').val(),
 partnercode: $('#partnercode').val(),
 office: $('#office').val(),
 // few more
 

 let error = ""
 if(criteria.company == "")
 
 error += "No company specified";
 
 if(criteria.partnercode == "")
 
 error += "No partner code specified";
 

 if(error == "") 
 postData(criteria);
 else 
 showModal(error);
 



function postData(criteria)
 $.post('process/editUser.php', criteria:criteria,(data)=>checkData())


function checkData(data) 
 if(data.indexOf("Error") >= 0) 
 showModal("error " + data, false);
 else 
 showModal("data posted!", true);
 


function showModal(content, button) 

 $('#modal').show();
 $('#message').text(content);
 if(button)
 $('#modalbutton').show();
 $('#modalbutton').on('click',() => hideModal());
 


function hideModal 
 $('#modal').hide();

The obvious improvement to make it less verbose, if all your if statement just check an empty string (that's why it's asked to post all your code here btw ^^), is to use a loop instead, for example :

for(let attr in criteria)
 if(criteria[attr] == "")
 
 $('#compError').show();
 return false;
 

Also, since return false; shut the current function execution, you don't need an else around your $post (and it wouldn't fit with this new loop system anyway).

You can set a class for each field that is required. Then, on form validate, loop through all the required fields. If you find one that is empty, go into your routine. Here's an example, which assumes that all required elements have the class required:

function validate() 
 var q = document.querySelectorAll('.required');
 for (var i=0; i<q.length; i++) 
 if ('' == q[i].value) 
 $('#compError').show();
 return false; 
 
 
 return true;

From a UX standpoint, I would consider focusing on the offending element, and showing a message saying that the specific field is required.

The question originally was tagged with functional-programming, but it seems more declarative/procedural than functional, other than the fact that callback functions are passed to the click handlers.

The answer by Nomis is good: one can iterate over the values in criteria and as soon as a blank value is found, show the error message container. A functional approach for this would be to use Object.values() to get the array of values in criteria and then utilize Array.every() to look for empty values:

var allNonEmpty = Object.values(criteria).every(function(entry) 
 return entry.length;
);
if (!allNonEmpty) 
 $('#compError').show();

If you are supporting ES-2015 (A.K.A. es-6) then an arrow function can make that shorter:

const allNonEmpty = Object.values(criteria).every(entry => entry.length);

It is also advisable to cache DOM lookups instead of performing them each time. So when the page loads, it might be best to store a reference to each input in a variable (or, if using es-6, a constant using const). See the expandable snippet below for an example.

$(function() //jQuery DOM-ready callback
 var inputFields = 
 company: $('#company'),
 partnercode: $('#partnercode'),
 office: $('#office')
 ;
 var compError = $('#compError');
 $('#saveSubmit').on('click', function() 
 var allNonEmpty = Object.values(inputFields).every(function(inputField) 
 return $(inputField).val().length;
 );
 if (!allNonEmpty) 
 compError.show();
 return;
 
 else 
 compError.hide();
 //AJAX code
 
 );
);

.hidden 
 display: none;

<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.1.1/jquery.min.js"></script>
<div id="banner-message">
 <pThe Form</p>
 <div>
 Company: <input id="company" />
 </div>
 <div>
 Partner: <input id="partnercode" />
 </div>
 <div>
 office: <input id="office" />
 </div>
 <button id="saveSubmit">submit</button>
 <div id="compError" class="hidden">
 Error!
 </div>
</div>