0

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.

Improve this question
5
  • Have a look at ternarys. Commented Mar 17, 2018 at 19:58
  • There are many validation libraries available in online. Try to use those instead of writing custom solution. Commented Mar 17, 2018 at 19:59
  • @JonasW. Next time just say ternarys are simply (condition) ? (return if true) : (return if false) and are most commonly expressed in variables such as var e = (true) ? 'a' : 'b'. They're simple, and why have somebody spend 5-10 minutes researching when they can be explained in a singular sentence? Commented Mar 17, 2018 at 20:00
  • 2
    @Ty cause then they might find it on their own the bext time :) Commented Mar 17, 2018 at 20:12
  • Fair point. @JonasW. Commented Mar 17, 2018 at 20:15

4 Answers 4

Reset to default
1

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.

Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

1

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")
 )
Improve this answer

Comments

0

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="[email protected]" />

    <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;
}

Improve this answer

Comments

0

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...

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.