javascript "if" statement returning false value when both values are equal

Question

I was trying out an HTML/JS login system, (which was only a learning experience, not to be used in a real case) There is an if statement that should return true when someone entered the correct username and password, but it returned false, and I can't figure out why. here is the code i used:

var count = 2;

function validate() {
  var un = document.login.username.value;
  var pw = document.login.password.value;
  var valid = false;
  var usernameArray = ["Adam", "Adam2"];
  var passwordArray = ["12345", "54321"];
  for (var i = 0; i < usernameArray.length; i++) {
    if ((un == usernameArray[i]) && (pw == passwordArray[i])) {
      valid = true;
      break;
    }
    if (valid) {
      alert("Logging in...");
      window.location.href = "http://www.google.com";
      return false;
    }
    var again = " tries";
    if (count == 1) {
      again = " try";
    }
    if (count >= 1) {
      alert("Username and password do not match.");
      count--;
    } else {
      alert("Username and password do not match. You are now blocked.")
      document.login.username.value = "You are now blocked";
      document.login.password.value = "You can not see this";
      document.login.username.disabled = true;
      document.login.password.disabled = true;
      return false;
    }
  }
}

<div class="container">
  <div class="header">
    <h1>Login Page</h1>
  </div>

  <form name="login" onsubmit="return validateForm();" method="post">

    <ul>
      <li>Username:
        <input class="username" type="text" name="username">
      </li>

      <li>Password:
        <input class="password" type="password" name="password">
      </li>
    </ul>

    <input type="button" class="submit" value="Login" name="submit" onclick="validate()">
  </form>
</div>

Have you [checked your console for errors?](http://stackoverflow.com/documentation/javascript/185/getting-started-with-javascript/714/using-console-log) — Mike Cluck, Nov 11 '16 at 20:59
Having the second `value = true;` outside of the `if` statement will consider any user, whether or not the credentials match, to be valid. I hope that's for testing as I don't think that's what you want. — War10ck, Nov 11 '16 at 20:59
@KevinB lets your code rest for a while and go for a coffee. Why? — VLAZ, Nov 11 '16 at 21:00
you could probably eliminate the need for the loop by using .indexOf() — Kevin B, Nov 11 '16 at 21:07

score 1 · Accepted Answer · edited Nov 11 '16 at 21:11

1

You are breaking the loop and your if(valid) code is within the for loop. You probably meant to have the if(valid) outside the for loop scope.

For example you can do:

valid = false;

for (var i = 0; i < usernameArray.length; i++) {
    if ((un == usernameArray[i]) && (pw == passwordArray[i])) {
        valid = true;
        break;
    }
}

if (valid) {
    alert("Logging in...");
    window.location.href = "http://www.google.com";
    return false;
}

Notice I closed the for loop.

Also notice you have an valid=true after the if statement. I assume you did it for debugging purposes, but make sure to remove it.

edited Nov 11 '16 at 21:11

War10ck

12,387
7
41
54

answered Nov 11 '16 at 21:01

Nuno_147

2,817
5
23
36

`for (var i = 0, len = usernameArray.length; i < len; i++) {` Slightly more efficient to pre-calculate the length of the array rather than calculating it on each iteration... – War10ck Nov 11 '16 at 21:12
@War10ck not really. It's not really that expensive to begin with unless you modify the length as you go along. Given that you _cache_ it, I don't expect you to be doing that. It's only an "issue" in older IE browsers. – VLAZ Nov 11 '16 at 21:28
@vlaz Not sure what your caching. It would just be storing the length. And I agree, in this case it's not a hugely significant difference, and visually you won't notice any speed increase, but it would still be more efficient to pre-caclulate the array length anyway... – War10ck Nov 14 '16 at 17:54
@War10ck caching the length of the array. Since you're saving it. However, implicitly, array length _won't_ change if you're doing that. The "length property is expensive" myth has long been debunked. In older IE browsers it _is_ but not in any sensible ones: http://stackoverflow.com/questions/17989270/for-loop-performance-storing-array-length-in-a-variable http://stackoverflow.com/questions/9592241/javascript-is-the-length-method-efficient http://stackoverflow.com/questions/5752906/is-reading-the-length-property-of-an-array-really-that-expensive-an-operation – VLAZ Nov 14 '16 at 23:15

score 1 · Answer 2 · answered Nov 11 '16 at 21:06

Its in your for loop. The logic after the valid credentials check is at a wrong place, it should be out of the for loop.

var count = 2;

function validate() {
  var un = document.login.username.value;
  var pw = document.login.password.value;

  var valid = false;
  var usernameArray = ["Adam", "Adam2"];
  var passwordArray = ["12345", "54321"];
  for (var i = 0; i < usernameArray.length; i++) {
    if ((un == usernameArray[i]) && (pw == passwordArray[i])) {
      valid = true;
      break;
    }
  }

  if (valid) {
    alert("Logging in...");
    window.location.href = "http://www.google.com";
    return false;
  }
  var again = " tries";
  if (count == 1) {
    again = " try";
  }
  if (count >= 1) {
    alert("Username and password do not match.");
    count--;
  } else {
    alert("Username and password do not match. You are now blocked.")
    document.login.username.value = "You are now blocked";
    document.login.password.value = "You can not see this";
    document.login.username.disabled = true;
    document.login.password.disabled = true;
    return false;

  }
}

<div class="container">
  <div class="header">
    <h1>Login Page</h1>
  </div>

  <form name="login" onsubmit="return validateForm();" method="post">

    <ul>
      <li>Username:
        <input class="username" type="text" name="username">
      </li>

      <li>Password:
        <input class="password" type="password" name="password">
      </li>
    </ul>

    <input type="button" class="submit" value="Login" name="submit" onclick="validate()">
  </form>
</div>

lovasoa · Answer 3 · 2016-11-11T21:15:25.117

-3

It is a very bad idea to implement login in javascript on the client side. Anyone can see your passwords.

That said, your problem is that you exit the loop as soon as you find a matching username and password. So your if statement is never reached.

edited Nov 11 '16 at 21:15

answered Nov 11 '16 at 21:00

lovasoa

6,419
1
35
45

`` The `validate()` function should run when the `onsubmit` event is fired, prior to the form attempting to submit to the server... – War10ck Nov 11 '16 at 21:14
how do you suggest i do it? – Ninjakid Nov 11 '16 at 21:16
@Ninjakid At a minimum, I'd say you need to use a server side language (i.e. PHP, Java, .NET, Ruby, Python, Perl, etc.) using a form that posts to the server over a secure SSL/TLS (i.e. https) connection... – War10ck Nov 11 '16 at 21:18
Ninjakid, as @War10ck said, you should really create a server-side login script, that you would query from your js code in order to test if authentication was successful. But anyway, keep in mind that someone will always be able to read and modify the js code that is running on his machine. – lovasoa Nov 11 '16 at 21:26

javascript "if" statement returning false value when both values are equal

3 Answers3