Hi, i’m trying to make a password strength checker, that checks a password and says if its strong,good,fair or weak. It seems to work, though im not sure if i approached it correctly. I have a bunch of if and if else statements that seem to be difficult to read, is there any way to make it better?
my code:
function passwordValidator(password) {
const hasUppercase = /(?=.*[A-Z])/;
const hasDigit = /(?=.*\d)/;
const hasSymbol = /[-!$#!@%^&*()_+|~=`{}\[\]:";'<>?,.\/]/;
const passwordLength = [7, 12];
if (!password) {
return "";
}
if (!hasUppercase.test(password)) {
return { name: "weak", suggestion: "Need at least one uppercase letter" };
} else {
if (
(password.length >= passwordLength[1] && hasDigit.test(password)) ||
(password.length >= passwordLength[1] && hasSymbol.test(password))
) {
return { name: "strong" };
} else if (
password.length >= passwordLength[0] &&
password.length <= passwordLength[1] &&
hasDigit.test(password)
) {
return { name: "good", suggestion: "Add at least one symbol" };
} else if (
password.length >= passwordLength[0] &&
password.length <= passwordLength[1] &&
hasSymbol.test(password)
) {
return { name: "good", suggestion: "Add at least one digit" };
} else if (password.length >= passwordLength[0]) {
return { name: "fair", suggestion: "Add at least one digit or a symbol" };
} else {
return { name: "weak" };
}
}
}
Since if the fist if is true, it will return and never hit the else. So, we can remove nesting with:
const myFunc = x => {
if (x > 100) {
return "big number";
}
if (x > 100) {
return "small number";
}
return "average number";
}
This will do the exact same thing, but I think is much easier to read without the nesting. you’re doing with with your first test for an empty string. I think if you applied it elsewhere, it gets more readable.
@kevinSmith thanks a lot dude, that makes a lot of sense. Switch case approach is pretty nice, but I don’t know if it’s ok when my statement is this long (password.length >= passwordLength[1] && hasDigit.test(password)) || (password.length >= passwordLength[1] && hasSymbol.test(password))
In fact it doesn’t even look good in the if statement, is it just because i’m not used to seeing that much code in the if statement or am I doing it incorrectly?
if ( password.length >= passwordLength[0]) {
if (...) {
...
}
}
that would repeat that expression only once, and put all the cases for which that has to be true, inside it
also, always remember that the first true condition is the one executed, if you order them in the right way, you can reduce the length of the statements