Can this be shortened/improved? I'm trying to make a password checker in python.
Could the if's be put into a for loop? And if so, how?
pw = input("Enter password to test: ")
caps = sum(1 for c in pw if c.isupper())
lower = sum(1 for c in pw if c.islower())
nums = sum(1 for c in pw if c.isnumeric())
scr = ['weak', 'medium', 'strong']
r = [caps, lower, nums]
if len(pw) < 6:
print("too short")
elif len(pw) > 12:
print("too long")
if caps >= 1:
if lower >= 1:
if nums >= 1:
print(scr[2])
elif nums < 1:
print("your password is " + scr[1])
elif lower < 1:
print("your password strength is " + scr[0])
elif caps < 1:
print("your password strength is " + scr[1])
Thanks for any suggestions :D
caps = sum(1 for c in pw if c.isupper())
can be:
caps = sum(c.isupper() for c in pw)
if caps >= 1:
can be:
if caps:
The most significant improvement: The bottom if/elif block can be completely removed by doing
i_strength = sum(map(bool,[caps,lower,nums])) - 1 #or sum(map(bool,r)) - 1
print('your password is {}'.format(scr[i_strength]))
Explanation: map(bool,[caps,lower,nums]) accumulates how many times each of caps,lower,nums is non-zero. Adding them up with sum gives you your "strength", which you've conveniently already put into a list, which can be accessed by index.
All of these improvements leverage the concept of "falsiness" in python, otherwise known as an object's value in a boolean context. Generally empty and zero things are False, and summing booleans is equivalent to adding ones and zeroes, so there you go.
Of course, it doesn't seem that you're doing anything with the counts of upper/lower/nums other than checking if they're nonzero. So a cleanup would just be
caps = any(c.isupper() for c in pw)
...
and then
i_strength = sum([caps,lower,nums]) -1
I would fix that nested if statement.
scr = ['weak', 'medium', 'strong'] # if you want to keep this fine
# but I suggest you do something like this:
_WEAK = scr[0]
_MEDIUM = scr[1]
_STRONG = scr[2]
if caps >= 1 and lower >= 1 and nums >= 1:
print(_STRONG)
elif caps < 1:
print("your password strength is " + _MEDIUM)
elif lower < 1:
print("your password strength is " + _WEAK)
elif nums < 1:
print("your password is " + _MEDIUM)
I'll ignore the general question, "can this code be shortened or improved", because that's a question for Code Review. But you've also got a specific question in there:
Could the if's be put into a for loop? And if so, how?
They could, but you'd have to turn them into something that can be put inside an iterator, like functions, and I really don't think you want to in this case.
Let's start with a simpler example, with just a linear series of checks:
checks = [
((lambda caps, lower, num: caps >= 1 and lower >= 1 and nums >= 1), 2),
((lambda caps, lower, num: caps < 1), 1),
((lambda caps, lower, num: lower < 1), 0),
((lambda caps, lower, num: num < 1), 1)
]
for check, value in checks:
if check(caps, lower, num):
print('your password strength is ' + scr[value])
break
You could instead put the check conditions in some encoded data form, and replace the if check(…) with a data-driven check on the conditions. For example:
checks = [
((1, 1, 1), 2),
((-1, 0, 0), 1),
((0, -1, 0), 0),
((0, 0, -1), 1)
]
for check, value in checks:
for value, condition in zip((caps, lower, num), check):
if condition == -1 and value >= 1 or condition == 1 and value < 1:
break
else:
print('your password strength is ' + scr[value])
break
But I think that's even less readable. There are plenty of use cases where this kind of thing makes sense—e.g., imagine you wanted to evaluate 40 polynomials for each value of x; you'd store each polynomial as a list of coefficients, and have generic "evaluate polynomial" logic like this. But this isn't one of those cases.
Either way, that's already pretty ugly. And if you want nested checks, you're going to need a nested structure, which you probably want to process recursively.
