CODEWITHSUNDEEP
javascript
Loodie
Loodie

Reputation: 1

How can I use this in a loop?

I'm doing this for a class assignment and I know there has to be a better way of writing it. Maybe some kind of loop that gets the inputs and labels? I'm repeating a lot here and it seems better to minify this if possible.

function checkEmptyFields() {

if(formName.value === "") {        
    formLabels[0].classList.add("has-errors");
    formLabels[0].innerHTML = "Name is required *";
    formName.style.borderBottomColor = "red";
} else {
    formLabels[0].classList.remove("has-errors");
    formLabels[0].innerHTML = "Name";
    formName.style.borderBottomColor = "green";
}

if(formEmail.value === "") {
    formLabels[1].classList.add("has-errors");
    formLabels[1].innerHTML = "Email is required *";
    formEmail.style.borderBottomColor = "red";
} else {
    formLabels[1].classList.remove("has-errors");
    formLabels[1].innerHTML = "Email";
    formEmail.style.borderBottomColor = "green";
}

if(formNumber.value === "") {
    formLabels[2].classList.add("has-errors");
    formLabels[2].innerHTML = "Phone is required *";
    formNumber.style.borderBottomColor = "red";
} else {
    formLabels[2].classList.remove("has-errors");
    formLabels[2].innerHTML = "Phone";
    formNumber.style.borderBottomColor = "green";
}

if(formMessage.value === "") {
    formLabels[3].classList.add("has-errors");
    formLabels[3].innerHTML = "message is required *";
    formMessage.style.borderBottomColor = "red";
} else {
    formLabels[3].classList.remove("has-errors");
    formLabels[3].innerHTML = "Email";
    formMessage.style.borderBottomColor = "green";
}
}

Upvotes: 0

Views: 128

Answers (5)

user7660113
user7660113

Reputation: 59

You can try like this:

fields = [{
 'name': formName,
 'index': 0,
 'css-error': "has-errors",
 'innerHtml': "Name",
 'innerHtml-error': "Name is required *",
 'borderBottomColor ': "green", //Or you can hardcode it in the loop itself.
 'borderBottomColor-error': "red"
},
 ....
]

for(var i=0; i < fields.length; i++) {
 var field = fields[i];
 if(field.name.value == "") {
   formLabels[field.index].classList.add(field.css);
   formLabels[field.index].innerHTML = field.innerHtml-error;
   field.name.style.borderBottomColor = field.borderBottomColor-error ;
 } else {
   formLabels[field.index].classList.remove(field.css);
   formLabels[field.index].innerHTML = field.innerHtml;
   field.name.style.borderBottomColor = field.borderBottomColor ;
 }
}

Upvotes: 2

Scott Marcus
Scott Marcus

Reputation: 65806

Anytime you have roughly the same code in more than one place, you should stop and rethink your approach as you are doing here.

If you give each of the HTML elements that need to be validated a common class, you can then create a node list (collection/array) that contains them. Then you can loop over that collection and perform the same test (written only once) on each item and act accordingly.

Also, I'm not quite sure what you are doing with .innerHTML, but don't use that when the text you are working with doesn't have any HTML in it. .innerHTML has security and performance implications. Instead, use .textContent when there is no HTML.

// Get all the form fields that need validation into an Array
let fields = Array.prototype.slice.call(document.querySelectorAll(".validationNeeded"));

// Set up form submit event handler
document.querySelector("form").addEventListener("submit", checkEmptyFields);

function checkEmptyFields(event) {
  let validCount = fields.length; // Holds the number of valid fields

  // Loop over the array
  fields.forEach(function(field){
    if(field.value === ""){
      field.previousElementSibling.classList.add("has-errors-label");    // style the label
      field.classList.add("has-errors-field"); // style the field
      field.classList.remove("valid-field");   // style the field        
      validCount--; // Decrease the count of valid fields
    } else {
      field.previousElementSibling.classList.remove("has-errors-label"); // style the label
      field.classList.remove("has-errors-field"); // style the field
      field.classList.add("valid-field");         // style the field       
    }
  });
  
  // Check to see if the form should be submitted
  if(validCount !== fields.length){
    event.preventDefault();  // Cancel the form's submission
  }
}
.row {margin-bottom:5px; }
.has-errors-label { color:red; }
.has-errors-field { outline:1px solid red; }
.valid-field      { outline:1px solid green; }
<form action="#" method="get">
  <div class="row">
    <label for="userName">Name: </label>
    <input class="validationNeeded" name="userName" id="userName">
  </div>
  <div class="row">
    <label for="email">Email: </label>
    <input class="validationNeeded" name="email" id="email">
  </div>
  <div class="row">
    <label for="phone">Phone: </label>
    <input class="validationNeeded" name="phone" id="phone">
  </div>
  <button>Submit</button>
</form>

Upvotes: 0

Peter B
Peter B

Reputation: 24137

You can create arrays for both the controls and the control names, and process them together with the formLabels array that you already have, in a for-loop that goes from 0 to length (not inclusive), like this:

function checkEmptyFields() {

    var controls = [formName, formEmail, formNumber, formMessage];
    var controlNames = ["Name", "Email", "Phone", "Message"];

    for (var i = 0; i < controls.length; i++) {
        if(controls[i].value === "") {        
            formLabels[i].classList.add("has-errors");
            formLabels[i].innerHTML = controlNames[i] + " is required *";
            controls[i].style.borderBottomColor = "red";
        } else {
            formLabels[i].classList.remove("has-errors");
            formLabels[i].innerHTML = controlNames[i];
            controls[i].style.borderBottomColor = "green";
        }
    }
}

Upvotes: 1

PumpkinBreath
PumpkinBreath

Reputation: 915

If you know about and understand for loops you can simply loop over 2 arrays of data like this:

function checkEmptyFields() {

    formArray = [formName, formEmail, formNumber, formMessage];
    labelArray = ["Name", "Email", "Phone", "Message"];

    for (let i = 0; i < formArray.length; i++) {
        if(formArray[i].value === "") {        
            formLabels[i].classList.add("has-errors");
            formLabels[i].innerHTML = labelArray[i] + " is required *";
            formArray[i].style.borderBottomColor = "red";
        } else {
            formLabels[i].classList.remove("has-errors");
            formLabels[i].innerHTML = labelArray[i];
            formArray[i].style.borderBottomColor = "green";
        }
    }
}

if not then you can read about them here:

https://www.w3schools.com/js/js_loop_for.asp

Upvotes: 0

Vasya
Vasya

Reputation: 469

You can write an additional function to check one field:

    function checkEmptyField(field, ind, msg, errmsg) {

    if(field.value === "") {        
        formLabels[ind].classList.add("has-errors");
        formLabels[ind].innerHTML = errmsg;
        field.style.borderBottomColor = "red";
    } else {
        formLabels[ind].classList.remove("has-errors");
        formLabels[ind].innerHTML = msg;
        field.style.borderBottomColor = "green";
    }
    }

Then you can call it

function checkEmptyFields() {
 checkEmptyField(formName,0,"Name","Name is required *");
...

Upvotes: 0

Related Questions