Skip to main content
deleted 1 character in body; added 2 characters in body
Source Link
200_success
  • 145.7k
  • 22
  • 191
  • 481

The & (binary bitwise AND) operator is not quite appropriate here. The and (boolean AND) operator would be more appropriate. Even though the results appear identical, the execution isn't —differs: the logical and allows short-circuit evaluation.

The & (binary bitwise AND) operator is not quite appropriate here. The and (boolean AND) operator would be more appropriate. Even though the results appear identical, the execution isn't — the logical and allows short-circuit evaluation.

The & (binary bitwise AND) operator is not quite appropriate here. The and (boolean AND) operator would be more appropriate. Even though the results appear identical, the execution differs: the logical and allows short-circuit evaluation.

deleted 4 characters in body; added 1 character in body; added 169 characters in body
Source Link
200_success
  • 145.7k
  • 22
  • 191
  • 481

The check_dict isn't doing anything for you. You'd be no worse off with five boolean variables. You are also calling each validation function twice.

The & (binary bitwise AND) operator is not quite appropriate here. The and (boolean AND) operator would be more appropriate. Even though the results appear identical, the execution isn't — the logical and allows short-circuit evaluation.

It is customary to put if __name__ == '__main__': around the any statements in the module that are not inside a function. That way, you could incorporate the functions into another program by doing import psk_validate without actually running thethis program.

The check_dict isn't doing anything for you. You'd be no worse off with five boolean variables.

The & (binary bitwise AND) operator is not quite appropriate here. The and (boolean AND) operator would be more appropriate.

It is customary to put if __name__ == '__main__': around the any statements in the module that are not inside a function. That way, you could incorporate the functions into another program by doing import psk_validate without actually running the program.

The check_dict isn't doing anything for you. You'd be no worse off with five boolean variables. You are also calling each validation function twice.

The & (binary bitwise AND) operator is not quite appropriate here. The and (boolean AND) operator would be more appropriate. Even though the results appear identical, the execution isn't — the logical and allows short-circuit evaluation.

It is customary to put if __name__ == '__main__': around the statements in the module that are not inside a function. That way, you could incorporate the functions into another program by doing import psk_validate without actually running this program.

Source Link
200_success
  • 145.7k
  • 22
  • 191
  • 481

Concept

Obligatory XKCD comic, before I begin:

XKCD Password Strength

Enforcing password strength by requiring human-unfriendly characters is no longer considered good practice. Nevertheless, I'll review the code as you have written it.

"Obvious" simplifications

  • Any code with the pattern if bool_expr: return True; else: return False should be written simply as return bool_expr.

  • Strings are directly iterable; there is no need to convert them into a list first, using .split(). In other words, the code would work the same if you just wrote:

     upper_list = "ABCDEFGHIJKLMNOPQRSTUVWXYZ" 
  • Better yet, you could just use string.ascii_uppercase.

  • The uppers += 1 counting loop could be written more expressively using the sum() built-in function. Actually, in this case, since you only care whether uppers > 0, you could just use the any() function.

With those changes, your check_upper() function becomes a one-liner:

def contains_upper(s): return any(c in ascii_uppercase for c in s) 

I've renamed check_upper() to contains_upper() to make it clear that the function returns True or False. Also, avoid using variable names, like input, that coincide with names of built-in functions: it could cause trouble if you ever want to use input().

Code duplication

Most of your check_something() functions are identical. You should generalize, instead of duplicating the code.

from string import ascii_uppercase, ascii_lowercase, digits def contains(required_chars, s): return any(c in required_chars for c in s) def contains_upper(s): return contains(ascii_uppercase, s) def contains_lower(s): return contains(ascii_lowercase, s) def contains_digit(s): return contains(digits, s) def contains_special(s): return contains(r"""!@$%^&*()_-+={}[]|\,.></?~`"':;""", s) def long_enough(s): return len(s) >= 8 

Note that I've used a raw long string to help deal with the need for backslashes in the punctuation string.

validate_password()

The check_dict isn't doing anything for you. You'd be no worse off with five boolean variables.

The & (binary bitwise AND) operator is not quite appropriate here. The and (boolean AND) operator would be more appropriate.

Personally, I'd write it this way, gathering up a list of all of the failure messages:

def validate_password(password): VALIDATIONS = ( (contains_upper, 'Password needs at least one upper-case character.'), (contains_lower, 'Password needs at least one lower-case character.'), (contains_digit, 'Password needs at least one number.'), (contains_special, 'Password needs at least one special character.'), (long_enough, 'Password needs to be at least 8 characters in length.'), ) failures = [ msg for validator, msg in VALIDATIONS if not validator(password) ] if not failures: return True else: print("Invalid password! Review below and change your password accordingly!\n") for msg in failures: print(msg) print('') return False 

If the function returns True in one place, then it would be good practice to return False instead of None in the other branch, for consistency.

Free-floating code

It is customary to put if __name__ == '__main__': around the any statements in the module that are not inside a function. That way, you could incorporate the functions into another program by doing import psk_validate without actually running the program.

Calling sys.exit(0) is rarely desirable or necessary, if you structure the code properly. Here, all you needed was a break.

if __name__ == '__main__': while True: password = raw_input("Enter desired password: ") print() if validate_password(password): print("Password meets all requirements and may be used.\n") print("Exiting program...\n") break