CODEWITHSUNDEEP
python
xero657
xero657

Reputation: 23

Function to validate pin code, why doesn't this code work?

So I have been trying to teach myself how to do this stuff and have had marginal success so far, I'm working on an exercise I found on codewars.com and I'm kind of stuck. The goal is to make a function that verifies the inputted pin meets the requirements, those requirements being: PIN must be either 4 or 6 digits long and PIN must be strictly numerical

I ran into a number of problems with it and ultimately came to the code below:

import re
def validate_pin(pin):
    for x in pin:
        if x.isalpha():
            return False
        elif re.match(".", pin):
            return False
    if len(pin) == 4 or len(pin) == 6:
        return True
    else:
        return False

I really, really, really, didn't want to ask for help but I have absolutely positively no idea why the above code doesn't work. It returns false on "1234" as input and I have racked my head over this for 2 days now. I refreshed myself on for loops, read up on regex, I tried searching forums but am completely stumped on what to look for. Any help anyone can provide would be greatly and deeply appreciated.

Edit: Wow! I didn't expect such positive feedback I appreciate it everyone! Thanks for helping me understand I have seen where I went wrong and will study it extra hard. Thanks for all the feedback!

Upvotes: 2

Views: 9573

Answers (4)

mehdi
mehdi

Reputation: 19

I think we can sort it without import, like this :

def validate_pin(pin):
    return len(pin) in (4, 6) and pin.isdigit()

Upvotes: 0

Moses Koledoye
Moses Koledoye

Reputation: 78564

Since your

PIN must be strictly numerical

This should not constitute something that will need a regex:

def validate_pin(pin):
    return len(pin) in (4, 6) and all(p in '0123456789' for p in pin)

Upvotes: 3

illright
illright

Reputation: 4043

There is a simpler way to validate it using regex. The reason why you're getting False returned on "1234" is because of that regex match. A point (.) matches ALL characters except for newline, so that match will almost always succeed. That's not what you want.

Here is a simpler solution I talked about earlier:

import re
def validate_pin(pin):
    if re.fullmatch("\d{4}|\d{6}", pin):
        return True
    else:
        return False

This will only work in Python 3.4 and above, since the fullmatch function was added in 3.4
If you're using an earlier version, use re.match("\d{4}$|\d{6}$", pin) instead

Some regex explanation:
This character "\d" matches only digits. The number in curly brackets that follows it shows exactly how many of the preceding token ("\d" in our case) we want to match. This "|" acts like an OR, telling that we can either match 4 digits, or 6 digits.
fullmatch will only return a match object if the entire string matches the pattern. But since we don't have that function in the earlier versions, this behaviour can be simulated by adding a "$" at the end of our pattern. It shows that we want this pattern to only match the end of the string and match function matches the characters from the beginning only. So combining these properties, we get a pattern that will only match the entire string

As suggested in the comments, to shorten the code, you can write the conditional like this:

def validate_pin(pin):
    return bool(re.fullmatch("\d{4}|\d{6}", pin))

If the match succeeded, the explicit conversion to bool will return True, if it didn't - it will return False

Upvotes: 7

Abdul Fatir
Abdul Fatir

Reputation: 6357

If you are allowed to use regex as I see from your code, why doesn't you use this regex ^(\d{4}|\d{6})$. 

import re
p = re.compile(ur'^(\d{4}|\d{6})$')
return re.match(p, pin)

Upvotes: 2

Related Questions