JS loop only works when matching to the last element in an array

Question

I cannot get the loop to work in my simple js login script. When i try to login with any login other than the last one in the array (user3 and pass3) it returns false.

What am I doing wrong?

I have tried both == and ===.

var userLogins = [{user:"user1", password:"pass1"},{user:"user2", password:"pass2"},{user:"user3", password:"pass3"}]
var success = null;
function logon(user, pass) {
    userok = false;
    for (i = 0; i < userLogins.length; i++)
    { 
        if(pass == userLogins[i].password && user == userLogins[i].user )
        {
            success = true;
        }
        else
        {
            success = false;
        }
    }
    secret(success);
}

function getData() {
    var user = document.getElementById("userid").value;
    var password = document.getElementById("password").value;
    logon(user, password);
}

function secret(auth){
    if(auth)
    {
        show('success');
        hide('login');
    }
    else
    {
        show('error');
        hide('login');
    }
}

function show(show) {
    document.getElementById(show).className = "show";
}
function hide(hide) {
    document.getElementById(hide).className = "hide";
}

Don't forget to declare i, for (var i = 0 otherwise trouble... — elclanrs
– elclanrs, Commented Jan 31, 2013 at 2:52
Nope. its for an assignment. I would never use JS like this for anything secure — coryrwest
– coryrwest, Commented Jan 31, 2013 at 22:14

Lightness Races in Orbit · Accepted Answer · 2013-01-31 03:23:44Z

3

for (i = 0; i < userLogins.length; i++)
{ 
    if(pass == userLogins[i].password && user == userLogins[i].user )
    {
        success = true;
    }
    else
    {
        success = false;
    }
}

You need a break in there, otherwise your true value for success simply gets overwritten with false on the next iteration... except for the last possible credentials, for which there is no "next" iteration.

Once you've done that, you don't actually need the else branch at all:

var success = false;
for (i = 0; i < userLogins.length; i++) { 
    if (pass == userLogins[i].password && user == userLogins[i].user) {
        success = true;
        break;
    }
}

answered Jan 31, 2013 at 3:23

Lightness Races in Orbit

387k77 gold badges670 silver badges1.1k bronze badges

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

Comments

Ricardo Lohmann · Accepted Answer · 2013-01-31 02:51:10Z

3

Use break when you found it. Otherwise the next loop will set success to false.

for (var i = 0; i < userLogins.length; i++)
{ 
    if(pass == userLogins[i].password && user == userLogins[i].user )
    {
        success = true;
        break;
    }
    else
    {
        success = false;
    }
}
secret(success);

answered Jan 31, 2013 at 2:51

Ricardo Lohmann

26.4k7 gold badges86 silver badges84 bronze badges

2 Comments

Jim Deville Over a year ago

Better yet, get rid of the else branch, set success = false at the beginning, and set success, call secret, and return out of the loop when you find a match

coryrwest Over a year ago

Simple! how did I not see that.

Collectives™ on Stack Overflow

JS loop only works when matching to the last element in an array

2 Answers 2

Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related