HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptModerate

Password strength checker

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
strengthcheckerpassword

Problem

I would like to receive feedback about my code. Is there any better way to shorten the code or is it fine?

Demo

var pass_strength;
function IsEnoughLength(str,length){
    if ((str == null) || isNaN(length))
        return false;
    else if (str.length = 3 && passwd.split(/[A-Z]/).length >= 3)
            return true;
        else
            return false;
    else
        return false;
}
function HasNumeral(passwd){
    if(passwd.match(/[0-9]/))
        return true;
    else
        return false;
}
function HasSpecialChars(passwd){
    if(passwd.match(/.[!,@,#,$,%,^,&,*,?,_,~]/))
        return true;
    else
        return false;
}
function CheckPasswordStrength(pwd)
{
    if (IsEnoughLength(pwd,14) && HasMixedCase(pwd) && HasNumeral(pwd) && HasSpecialChars(pwd))
        pass_strength = "Very strong";
    else if (IsEnoughLength(pwd,10) && HasMixedCase(pwd) && HasNumeral(pwd) && HasSpecialChars(pwd))
        pass_strength = "Strong";
    else if (IsEnoughLength(pwd,10) && HasMixedCase(pwd) && HasNumeral(pwd))
        pass_strength = "Medium";
    else
        pass_strength = "Weak";
    document.getElementById('pwd_strength').innerHTML = "Password strength: " + pass_strength;
}

Solution

-
You can replace all of the

if(x.match(...))
    return true;
else
    return false;


with:

return x.match(...) !== null;


or if you're content with returning a truthy/falsy value instead of proper booleans you can even simplify it to:

return x.match(...)


-
IsEnoughLength is ungrammatical. I'd use IsLongEnough

  • You're mixing the password strength estimation with presentation logic. You should have one function which returns a value indicating the strength of the password and another which manipulates the DOM.



  • Use CSS classes to format the text instead of ` and



-
Personally I wouldn't bother with argument checking in such a simple internal function. But if you do, throw an exception on programmer error, don't just return false.

if ((str == null) || isNaN(length))
    return false;


should be something like

if (str == null)
    throw new Error("str must not be null");
if(isNaN(length))
    throw new Error("length must be a number");


(or a fancier exception class if you want)

Code Snippets

if(x.match(...))
    return true;
else
    return false;
return x.match(...) !== null;
return x.match(...)
if ((str == null) || isNaN(length))
    return false;
if (str == null)
    throw new Error("str must not be null");
if(isNaN(length))
    throw new Error("length must be a number");

Context

StackExchange Code Review Q#48680, answer score: 10

Revisions (0)

No revisions yet.