Search code examples
pythonstringfunctionfor-loopreturn

Need insight into why my Python function isn't doing a proper comparison of strings


I'm completing an online course that utilizes Python 3 and below is the prompt I received:

Write a function called is_valid. is_valid should take two parameters: a string to check, and a string of all valid characters.

is_valid should return the boolean True if all the characters in the string to check are present in the string of valid characters. It should return False if any character in the checked string does not appear.

I have defined a function that takes in the two parameters mentioned in the problem, a string (which in my code I call string)), and a string of acceptable characters (which I called validcharacters).

I'm not supposed to use any of Python's built-in string functions, so I decided to write a for loop with my index variable being called character that indexes through string. After that for loop is written, I then wrote an if statement that looks to see if character (index variable) is in validcharacters. If it is, it returns True. Otherwise, I wrote an else statement which returns False.

def is_valid(string, validcharacters):
    for character in string:
        if character in validcharacters:
            return True
        else:
            return False

Below are some test cases to work with:

sample_valid_string = "1234-5678-9011-1111"
sample_invalid_string = "1234!5678.9011?1111"
valid_characters = "0123456789-"

print(is_valid(sample_valid_string, valid_characters))
print(is_valid(sample_invalid_string, valid_characters))

I expect the output (for the test cases I listed above) to be True and False respectively. However, the way my code is written, it returns True and True. I found out through debugging it only iterates through the first character of my code and fails to iterate the other characters. I'm not sure how I need to re-write my code in order to ensure it goes through other characters before returning True or False. Do I need to re-position my return statements? Are they causing the function to end too soon?


Solution

  • Given the way you've defined is_valid, you are directly returning True if the first character in string is in valid_characters. You should only be returning True once you've checked all characters are valid. You should instead be doing something like:

    def is_valid(string, validcharacters):
        for character in string:
            if character not in validcharacters:
                return False
        return True
    

    Note that from the docs, this is the proposed equivalent of all, which could be used to simplify the above to:

    def is_valid(string, validcharacters):
        return all(i in valid_characters for i in string)
    

    Where all will return True when all elements in the generator expression are also True.


    Returning in this case:

    print(is_valid(sample_valid_string, valid_characters))
    print(is_valid(sample_invalid_string, valid_characters))
    
    True
    False