CODEWITHSUNDEEP
javascriptjquery
Michael Surbakti
Michael Surbakti

Reputation: 1

Why javascript IF/ELSE doesn't work properly?

I'm new to JavaScript and I want to make username validation on registration form. I don't know what's wrong on my code but I think I have a problem with "IF ELSE" statement. 

$(document).ready(function() {
  $("#usernameErrorMsg").hide();
  var usernameLength = $("#username").length;
  $("#username").focusout(function() {
    checkUser();
  });
  function checkUser() {
    if (usernameLength < 5 || usernameLength > 20) {
      $("#usernameErrorMsg").show();
    } else {
      $("#usernameErrorMsg").hide();
    }
  }
});

I expected that when I input more than 5 char until 20 char, the usernameErrorMsg will disappear. The actual result, no matter how many characters that I have input, the error message keeps coming up.

Upvotes: 0

Views: 84

Answers (4)

Alexey Zelenin
Alexey Zelenin

Reputation: 730

try this

$(document).ready(function(){

$("#usernameErrorMsg").hide();

  $("#username").focusout(function(){
    checkUser();
  });

  function checkUser(){
    usernameLength = $("#username").length;
    if(usernameLength < 5 || usernameLength > 20){
      $("#usernameErrorMsg").show();
    }
    else{
      $("#usernameErrorMsg").hide();
    }
  }

});

Upvotes: -2

ShivCK
ShivCK

Reputation: 659

You need to get the length of input everytime the function checks it.

Currently you calculate the length only once.

$(document).ready(function(){

$("#usernameErrorMsg").hide();

  $("#username").focusout(function(){
    checkUser();
  });

  function checkUser(){
   //calculate the length everytime in the function
    var usernameLength = $("#username").val().length;

    if(usernameLength < 5 || usernameLength > 20){
      $("#usernameErrorMsg").show();
    }
    else{
      $("#usernameErrorMsg").hide();
    }
  }

Upvotes: 0

Scott Marcus
Scott Marcus

Reputation: 65873

$("#username").length is not the length of the characters in the field. It's the amount of elements in the JQuery wrapped set returned from your query. Since only one element will have an id of username, the length is always 1 and your if condition will always be true.

What you'll have to do is get the length of the value in the field:

$("#username").val().length

You'll also want to move the line that gets the value into the focusout event handler so that you are always working with the most current data.

Lastly, it doesn't make much sense to have a function do nothing but call another function, so you can combine the checkUser function with the event callback and simplify the code.

$(document).ready(function() {
  $("#usernameErrorMsg").hide();  
 
  var username = $("#username");        // Find the element just once
  
  $("#username").focusout(function() {
    // Each time the user leaves the field, get the current
    // amount of characters in the input field
    var usernameLength = username.val().length;
  
    // See the difference between the length of the JQuery query and 
    // the length of the value of the first element in the results?
    console.log(username.length, usernameLength);
    
    if (usernameLength < 5 || usernameLength > 20) {
      $("#usernameErrorMsg").show();
    } else {
      $("#usernameErrorMsg").hide();
    }
  });
});
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
Enter your name: <input id="username">
<span id="usernameErrorMsg">ERROR</span>

Upvotes: 1

Matt Ball
Matt Ball

Reputation: 360046

usernameLength is being computed only once, before checkUser() ever runs. You should re-calculate its value inside the callback each time; otherwise, changes to that value will not be visible inside the callback.

Furthermore, if you want to check the length of the test in the input, you need to check $("#username").val().length, not $("#username").length:

function checkUser(){
  var usernameLength = $("#username").val().length;
  if (usernameLength < 5 || usernameLength > 20){
    $("#usernameErrorMsg").show();
  } else {
    $("#usernameErrorMsg").hide();
  }
}

Upvotes: 3

Related Questions