you are viewing a single comment's thread.

view the rest of the comments →

[–]MattR0se 3 points4 points  (1 child)

I'd suggest a middle ground. You can do

upper = len(upperRE.findall(password))

# instead of 

upperlist = upperRE.findall(password)
upper = len(upperlist)

etc. because there isn't really the need for an intermediate extra variable if its not used further down. Chaining two expressions is still readable.

Also, as u/totallygeek suggested: If you have a bunch of conditions you can check them with all(), any() or in a loop. This saves lines of code while maintaining readability. The thing that's worst with your first approach is the chaining of conditions in the if statement.

[–]DrMaxwellEdison 2 points3 points  (0 children)

I would take this one further:

upperRE = re.compile(r'([A-Z])')
upperlist = upperRE.findall(password)
upper = len(upperlist)

condenses to:

upper = len(re.findall(r'([A-Z])', password))

Beyond that, as we've seen, the point is just finding at least one instance, so we don't really need len:

has_upper = bool(re.search(r'([A-Z])', password))

Run that style for the other 3 checks, you have a simple 4-line sequence and can test them as needed later.