CS50 Vanity plates

Question

I modified my code with respect to the suggestions given in the comments using regex as follows, but when all my characters are letters it gives out an output of "invalid" whereas it should not:

       import re


def main():
    plate = input("Plate: ")
    if is_valid(plate):
        print("Valid")
    else:
        print("Invalid")


def is_valid(s):
    n = re.match(r"^[a-zA-Z]{2,5}[1-9][0-9]*$", s)
    if length(s) and n:
        return True

def length(s):
    if 2<=len(s)<=6:
        return True
    else:
        return False

main()

Answer 1

Second Update from your Comments
For punctuation you can simplify to a single python function

def punctuation(s):
    # returns true is s is all alpha-numeric characters
    return s.isalnum()

Or we could build a regex.

For the numbers function your precheck of s.isalpha() should filter out the all letters case. if it is all numbers it will fail the regex match that i gave you.

Update from your edits
After your updates you have repaired MOST of the functions.
However you broke punction and number is still broken.

First punctuation let me explain your code and show you what you should have done.

# Your Original Code with additional comments
def punctuation(s):
    for i in s: #ok check every 'character'
        if i.isalpha() or i.isdigit():
            return True # wrong you just returned true if the first character is valid.
        else:
            return False # correct as soon as you see an invalid character you can stop.

# Here's a corrected version of `punctuation` which you basically had before you were only missing one thing.

def punctuation(s):
    for i in s:
        ## alternate form below in comment which will avoid calling is_digit when is_alpha is True
        # if not i.isalpha() and not i.isdigit():

        # note added parenthesis to be clear that the not should be applied to the or.
        if not (i.isalpha() or i.isdigit()):
            return False
    # note the return True is OUTSIDE the loop body
    return True

Now for number(s) unfortunately which was more correct in your original version

# Your current code with comments
def number(s):
    if s.isalpha(): 
        return True #ok
    else:
        n=re.search(r"\d+$", s)
        m=n.start()
        for b in range(m-1): # b isn't used, this loop is unneeded you just loop over the same thing for c in s multiple times, and if you find a digit anywhere you return false.  This is flat out wrong. I dont think this is doing what you intended it to do either.
            for c in s:
                if c.isdigit():
                    return False
        for index, i in enumerate(s): # no need to enumerate as you don't use index.  this can just be `for i in s:`
            if i.isdigit(): #this makes sense
                a=i  # you should introduce a outside of the loop if your want to retreive it outside the loop but ok..
                break
        if a=="0":
            return False  # this is a correct check
        elif a !=0 and n: # this is not correct for the same reason I described before
            return True

I'm not sure what it is you are trying to do at this point. I think you are attempting to check the characters before the match of digits but that isn't what you are actually doing, particularly if no match was found. (what you are doing now is repeatedly iterating over the whole word and returning false if you find a digit. which is wrong).

For the moment lets forget this piece and throw it out.

If you are going to use a regular expression why not let the regex do more for you?

First off you can vastly improve your mixed useage of regex and manual iteration by changing your regex to : n = re.search(r"[1-9][0-9]*$", s) this way it will auto fail for you if the first digit of the string of digits is 0. This still leaves you to look for a stray digit (0 or not) elsewhere in the string. However you could do this with a different regex. n = re.search(r"[a-zA-Z][0-9][a-zA-Z]", s) this will find any occurence of a digit being between two letters.

But this doesn't check that the first two characters are letters ( yes you technically have a seperate check for that... ) but what if we could combine that check, with the check for no digits in the middle, combined with no leading zeroes?

n = re.match(r"^[a-zA-Z]{2,5}[1-9][0-9]*$", s) This does everything for you except check string length.

• It makes sure the string starts with 2 to 5 letters (your previous check said there should be at least one number in this function )
• It makes sure that if there are digits they are at the end
  • That the digits are started with a non zero
  • That the non zero can be followed by any number of digits
• And then the string ends
• It makes sure there are no punctuation marks (removing need for the punctuation check)

The only check left outside of this is total length < 6.

Moral of story: If you are going to use regex, use regex. If you are going to scan the string manually, just scan the string manually. I wouldn't mix the two unless I absolutely had to. Which largely for me becomes in cases of look-ahead, look-behind, and combinations of pattern matching and capturing. This is a good case because I dont know the repetition number for the numbers as they are functions of the count of the letters present and still possibly optional, so as a result I would use a seperate string length calculation. But if I am going to use the regex on the string, in this case there is really no need to externally call isalpha and isdigit, other than the precheck of the whole string being alpha's to know i dont have to use this regexp to avoid having to use lookahead/lookbehinds. Now you can get rid of letter and punctuation functions.

your is valid function can just be a length check followed by the body of what is left of number which is basically just a check that its all letters OR that this regex is a match.

Original

Aside from main every one of your functions is broken.

How to go about this:

In short. Your is valid is the composition of a number of other checks. For each one of the checks I suggest you verify whether they are correctly working or not. For example just by altering is_valid we can get a better picture for debugging:

def is_valid(s):
    n = number(s)
    l = length(s)
    a = letter(s)
    p = punctuation(s)
    print("Number:", str(n), "\n",
          "Length:", str(l), "\n",
          "Letter:", str(a), "\n",
          "Punctuation:", str(p) )
    return n and l and a and p

Now for each input you can see exactly which subtest is causing it to be marked invalid.

Now some hints and pointers:

• You only have a return path for True or only for False this is bad.

• Example: is_valid should just be return number(s) and length(s) and letter(s) and punctuation(s) Though I would not use that ordering. No need for if but if you do an if to return True you need an else to return False.

• Same for letter and length.

• punctuation needs a return path to return true. when should it return True?

• Your number check is flat out broken. And frankly I would save the number check for last after you have performed the other checks.

• number function:

  • Why use enumerate? You are not using the value of index anywhere.
  • You break the loop on the first digit you find, if that digit is 0 you return false which is correct behavior.
  • if a isn't 0, you check for the n match. This is insufficient. Consider the license plate "AB1C23". It will have matched the regular expression but its still invalid. So you will mark some plates valid that should be invalid.
    • you need to tune the regex so that all characters before the digit are not digits.