Form validation using javascript using if-else condition

Question

I have done a javascript form validation using the following code.I'm not sure whether it is the correct way of validating form.

const signup=()=>{
    let name=document.querySelector("#u_name").value;
    let email=document.querySelector("#email_id").value;
    let password=document.querySelector("#pwd").value;
    let confirmPassword=document.querySelector("#confirm_pwd").value;
    let i=0;
    if((name==""||email=="")||(password==""||confirmPassword==""))
    {
        document.querySelector("#empty-field").innerHTML="*Fill all required fields";
        i++;
    }
    else
    {
    if(name.length<3)
    {
        document.querySelector("#u_name").style.borderColor="red";
        document.querySelector("#user-errmsg").innerHTML="*Enter valid user name";
        i++;
    }
    else
    {
        document.querySelector("#u_name").style.borderColor="#ced4da";
        document.querySelector("#user-errmsg").innerHTML="";
        i;
    }
    if(email.length<6)
    {
        document.querySelector("#email_id").style.borderColor="red";
        document.querySelector("#email-errmsg").innerHTML="*Enter valid email id";
        i++;
    }
    else
    {
        document.querySelector("#email_id").style.borderColor="#ced4da";
        document.querySelector("#email-errmsg").innerHTML="";
        i;
    }
    if(password.length<6 && confirmPassword.length<6)
    {
        document.querySelector("#pwd").style.borderColor="red";
        document.querySelector("#confirm_pwd").style.borderColor="red";
        document.querySelector("#pwd-errmsg").innerHTML="*Password must be atleast 6 digits long";
        i++;
    }
    else if(password.length<6 && confirmPassword.length>=6)
    {
        document.querySelector("#confirm_pwd").style.borderColor="red";
        document.querySelector("#pwd").style.borderColor="red";
        document.querySelector("#pwd-errmsg").innerHTML="*Password must be atleast 6 digits long";
        i++;
    }
    else if(password.length>=6 && confirmPassword.length>=6)
        {
            if(password!= confirmPassword)
            {
                document.querySelector("#pwd").style.borderColor="red";
                document.querySelector("#confirm_pwd").style.borderColor="red";
                document.querySelector("#pwd-errmsg").innerHTML="*Both fields must have the same password";
                i++;
            }
            else
            {
                document.querySelector("#pwd").style.borderColor="#ced4da";
                document.querySelector("#confirm_pwd").style.borderColor="#ced4da";
                document.querySelector("#pwd-errmsg").innerHTML="";
                i;
            }
        }
    else
    {
        document.querySelector("#pwd").style.borderColor="red";
        document.querySelector("#confirm_pwd").style.borderColor="red";
        document.querySelector("#pwd-errmsg").innerHTML="*Both fields must have the same password";
        i++;
    }
    document.querySelector("#empty-field").innerHTML="";
    }
    if(i==0)
    return true;
    else
    return false
}

Is it a good practice to write too many if else condition? If not, how can I rewrite it?

//ignore

Looks like stackoverflow doesn't allow posting this question with less details :/ So I have to add some more it seems.

Answer 1

You don't need any javascript validation or extra library, just use all the attributes that is needed on the fields and correct type. use "constraint validation". Only thing you need to check is if the password match (showing you how below)

Study the input attributes here: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input

function validatePassword(){
  const pwd1 = document.getElementById("pwd")
  const pwd2 = document.getElementById("confirm_pwd")
  
  pwd1.setCustomValidity(pwd1.value !== pwd2.value
    ? "Passwords Don't Match" 
    : ""
  )
}

document.getElementById("pwd").oninput = validatePassword
document.getElementById("confirm_pwd").oninput = validatePassword

input:not([type=submit]):valid {
  border: 1px solid lime;
}

<form class="" action="index.html" method="post">
  <br>name<br>
  <input name="" required type="text" autocomplete="name" minlength="3" id="u_name">
  <br>email<br>
  <input name="" required type="email" autocomplete="email" id="email_id">
  <br>pwd<br>
  <input name="" required type="password" autocomplete="new-password" minlength="6" id="pwd">
  <br>repeat pwd<br>
  <input name="" required type="password" autocomplete="new-password" minlength="6" id="confirm_pwd">

  <br><input type="submit">
</form>

Personally I feel very tired off repeating my email and/or password. If i get it wrong i will do it over again or press that "forgot your password" link. I can see my email address and with the help of autocomplete it's a lower risk i get my email wrong. You don't need that b/c every other website dose it. if that is out of the way you don't need any javascript at all...

Answer 2

use jquery plugin https://jqueryvalidation.org/

Below is the example to use:

$(function() {
  // Initialize form validation on the registration form.
  // It has the name attribute "myform"
  $("form[name='myform']").validate({
    // Specify validation rules
    rules: {
      firstname: "required",
      lastname: "required",
      email: {
        required: true,
        email: true
      },
      password: {
        required: true,
        minlength: 5
      }
    },
    // Specify validation error messages
    messages: {
      firstname: "Please enter your first name",
      lastname: "Please enter your last name",
      password: {
        required: "Please provide a password",
        minlength: "Your password must be at least 8 characters long"
      },
      email: "Please enter a valid email"
    },
    submitHandler: function(form) {
      form.submit();
    }
  });
});

<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery-validate/1.17.0/jquery.validate.min.js"></script>

<div>
  <h2>Sign Up</h2>
  <form action="" name="myform">

    <label for="firstname">First Name</label>
    <input type="text" name="firstname" id="firstname" placeholder="Robert" />

    <label for="lastname">Last Name</label>
    <input type="text" name="lastname" id="lastname" placeholder="Smith" />

    <label for="email">Email</label>
    <input type="email" name="email" id="email" placeholder="name@company.com" />

    <label for="password">Password</label>
    <input type="password" name="password" id="password" placeholder="" />

    <button type="submit">Submit</button>

  </form>
</div>

CSS to highlight error

label.error {
  color: red;
  margin-top:-10px;
  margin-bottom:15px;
}

Answer 3

Your code would benefit from extracting everything into functions:

  const get = selector => document.querySelector(selector);

 const checker = (check, msg) => (el, error) => {
  if(check(get(el).value)){
    get(el).style.color = "green";
    get(error).innerHTML = "";
    return true;
  } else {
    get(el).style.color = "red";
    get(error).innerHTML = msg;
  }
 };

 const minLength = length => checker(
   v => v.length >= length,
  `Too short! You need at least ${length} chars`
 );

 const maxLength = length => checker(
   v => v.length <= length,
  `Too long! You need less than ${length} chars`
 );

 const equal = (a, b, err) => checker(v => v === get(b).value, "They must be equal")(a, err);

Seems quite long right? But now you can do:

 return (
   minLength(6)("#u_name", "#user-errmsg") &&
   maxLength(12)("#u_name", "#user-errmsg") &&
   minLength(6)("#email_id", "#email-errmsg") &&
   equal("#confirm_pwd", "#pwd", "#pwd-errmsg")
 ) 

Answer 4

Your concern about using too many if/else statements during the scope of a single method is a valid one. It is not wrong, but makes the code hard to read/understand and difficult to debug/troubleshoot if something goes wrong. Here are some advices to refactor this method:

1. It seems that it isn't doing a signup. You're validating input data so I would recommend to rename it to validate or something similar.
2. Method is doing too much. It's querying for the data, it's performing validations and also rendering messages and adapting styles. I advice to divide and conquer. Make this method just a validation one.
3. Create small functions that performs a single validation. As an example validateEmailAddress() or validatePassword(). Once you start moving pieces around, you will have less if/elseif statements.

There are more things you can do but the key is on decoupling responsibilities. If you try that I believe your if/elseif amount will decrease.

There is another strategy that I use all the time to remove if/else nesting levels which is commonly called as early return.