CODEWITHSUNDEEP
pythonpython-3.x
bigez
bigez

Reputation: 63

Checking a password

I think I have the right idea of solving this function, but I'm not sure why I don't get the desired results shown in the docstring. Can anyone please help me fix this?

def check_password(s):
'''(str, bool)
>>> check_password('TopSecret')
False
>>> check_password('TopSecret15')
True
'''
for char in s:
    if char.isdigit():
        if  char.islower():
            if char.isupper():
                return True
else:
    return False

Upvotes: 1

Views: 667

Answers (4)

user8651755
user8651755

Reputation:

I've gone way overboard here (because I'm bored I guess) and whipped up an extensible password-checking system. Perhaps it will help clarify why your version doesn't do what you want (or maybe it won't).

The essential problem with your code is that it checks for a single characteristic instead each of the required characteristics.

def check_password(password):
    # These are the characteristics we want a password to have:
    characteristics = [
        length(8),      # At least 8 characters
        lower_case(1),  # At least 1 lower case letter
        upper_case(1),  # At least 1 upper case letter
        number(1),      # At least 1 number
    ]
    # Check to see if the supplied password has *all* of the desired
    # characteristics:
    return all(
        characteristic(password)
        for characteristic in characteristics
    )


def length(n=10):
    # Ensure password has at least N characters
    def check(password):
        return len(password) >= n
    return check


def lower_case(n=1):
    # Ensure password has at least N lower case characters
    def check(password):
        count = 0
        for char in password:
            if char.islower():
                count += 1
            if count == n:
                return True
        return False
    return check


def upper_case(n=1):
    # Ensure password has at least N upper case characters
    def check(password):
        count = 0
        for char in password:
            if char.isupper():
                count += 1
            if count == n:
                return True
        return False
    return check


def number(n=1):
    # Ensure password has at least N numbers
    def check(password):
        count = 0
        for char in password:
            if char.isdigit():
                count += 1
            if count == n:
                return True
        return False
    return check


# Doesn't have any numbers:
>>> print(check_password('TopSecret'))
False

# Acceptable:
>>> print(check_password('TopSecret15'))
True

# Has lower case, upper case, and number, but it's not long enough:
>>> print(check_password('Ab1'))
False

Notes:

  • Some parts of this are simplified in order to make it more clear what's going on (all of the check functions could be one-liners).
  • In a production system, password requirements will almost certainly change over time and you want to make sure that implementing additional requirements is easy.
  • A regular expression might not be as easy to understand or extend (although it might be a little faster, but that probably doesn't matter at all).
  • The approach above is somewhat similar to Django's password validation system.

Upvotes: 0

DoesData
DoesData

Reputation: 7037

Your logic is flawed it should look like this:

def check_password(s):
    has_digit = False
    has_lower = False
    has_upper = False

    for char in s:
        if char.isdigit():
            has_digit = True
        if char.islower():
            has_lower = True        
        if char.isupper():  
            has_upper = True 

    # if all three are true return true
    if has_digit and has_upper and has_lower:
        return True
    else:
        return False

Now, let's talk about what's wrong with your code. 

def check_password(s):
    for char in s:
        if char.isdigit():
            if char.islower(): # we only get to this check if char was a digit
                if char.isupper(): # we only get here if char was a digit and lower
                    # it is not possible to get here
                    # char would have to be a digit, lower, and upper
                    return True
    else:
        return False

As an example let's look at TopSecret15, we start with T

  1. isdigit = False so we immediately return false for 'T'
  2. We will continue to immediately return false until we get to '1'
  3. Let's say we were on 1, isdigit would be true, but islower would be false so again it returns false

Do you see how it is impossible for all three of these to be true for the same char?

Your code is equivalent to: 

if char.isdigit and char.islower and char.isupper:
    # this will never happen

Upvotes: 2

Shailyn Ortiz
Shailyn Ortiz

Reputation: 766

See this answer: https://stackoverflow.com/a/2990682/7579116 the best way to check if a password match all the requirements is by using a regex.

Upvotes: 0

Sebastian Mendez
Sebastian Mendez

Reputation: 2971

The reason this doesn't work is that for any given character, it first checks if it's a digit, then if it's lower, then if it's upper. Obviously, one character cannot be all three of these at once. You instead want to check each character to see if it's a digit, lowercase, or uppercase, and then flip a boolean value like has_upper. Then, after the for loop, you'll check to see if all of the boolean values are true.

Upvotes: 1

Related Questions