CODEWITHSUNDEEP
javascript
renchan
renchan

Reputation: 519

How to write loops for field validation

I'm trying to figure out why validation is always fails? Regular expressions should be fine, I guess the problem should be somewhere in if(loginreg.test(login) && passwordreg.test(password) && repassword==password). Could someone please help me with it?

function CheckAuthData() {  
    var loginreg = /^(?=.*[A-Z])(?=.*\d)[A-Za-z\d]{4,}\d$/g;
    var login = document.getElementById("login").value; 
    var passwordreg = 
        /^(?=.*?[A-Z])(?=.*?[a-z])(?=.*?[0-9])(?=.*?[#?!@$%^&*-]).{1,}$/g;
    var password = document.getElementById("password").value;
    var repassword = document.getElementById("repassword").value;    

    if(loginreg.test(login) && passwordreg.test(password) && repassword==password){         
        document.getElementById("labellogin").innerHTML = "";   
        document.getElementById("labelpassword").innerHTML = "";    
        document.getElementById("labelrepassword").innerHTML = "";      
        document.getElementById("pers_data").style.display = "block";       
    }

    if(!loginreg.test(login)){
        var label = document.getElementById("labellogin").innerHTML = "Login error!";
        document.getElementById("labellogin").style.color = "red";
    }   

    if(!passwordreg.test(password)){
        document.getElementById("labelpassword").innerHTML = "Password error!";
        document.getElementById("labelpassword").style.color = "red";
    }   

    if(repassword!=password){
        document.getElementById("labelrepassword").innerHTML =
            "Should be the same password!";
        document.getElementById("labelrepassword").style.color = "red";
    }
}

Upvotes: 0

Views: 46

Answers (1)

T.J. Crowder
T.J. Crowder

Reputation: 1074495

The only actual error I can see there is that if any of those conditions fails once, you never clear the red off the element where it happened. But since you clear the text out of the element, it probably doesn't matter.

That said, testing the same conditions twice (in your first if and then again in subsequent ones, inverted) is a maintenance problem waiting to happen, as is all of the repeated code in the function.

You also don't need to recreate the regular expressions every time the function is called, and you don't need the g flag on them.

This addresses the various issues above (note: I'm assuming this is contained in some kind of scoping function, so we're not creating new globals by moving those regexes out):

var loginreg = /^(?=.*[A-Z])(?=.*\d)[A-Za-z\d]{4,}\d$/;
var passwordreg = 
        /^(?=.*?[A-Z])(?=.*?[a-z])(?=.*?[0-9])(?=.*?[#?!@$%^&*-]).{1,}$/;
function CheckAuthData() {  
    var login = document.getElementById("login").value; 
    var password = document.getElementById("password").value;
    var repassword = document.getElementById("repassword").value;    
    var failed = false;

    updateField("labellogin", loginreg.test(login), "Login error!");
    updateField("labelpassword", passwordreg.test(password), "Password error!");
    updateField("labelrepassword", repassword == password, "Should be the same password!");

    if(!failed) {
        document.getElementById("pers_data").style.display = "block";       
    }

    function updateField(id, valid, message) {
        var element = document.getElementById(id);
        element.innerHTML = valid ? "" : message;
        element.style.color = valid ? "" : "red";
        failed = failed || !valid;
    }
}

Made this a CW because...well...it probably doesn't actually answer the question, because there isn't enough info in the question. But hopefully it's helpful.

Upvotes: 3

Related Questions