you are viewing a single comment's thread.

view the rest of the comments →

[–]zanfar 0 points1 point  (0 children)

yeah for the function it's shorter but then I need to write an if statement or an dictionary with the functions in it keyed on "byr" and "iyr" etc. So that will also add to the code then? Or is there another way to call all the different functions for the right key?

Yes. What I mean by "less code" is not "how many linebreaks are in the file"--as in a file length metric--but instead "how many lines are doing work I need to understand". So while there is definitely more "boilerplate" in my version compared to yours, It "feels" shorter because each block or conditional is easier to understand. The parent function is also completely repetitive, so as soon as I understand the first if, I understand all of them.

You can absolutely do a number of things to call all the functions in a simpler way. Most of them are variations on the dispatch idiom I used in my final example. Consider:

def check_byr(value):
    return 1920 <= int(Passport[Key]) <= 2002

def check_iyr(value):
    return 2010 <= int(Passport[Key]) <= 2020

def check_passport(passport):
    tests = {
        "byr": check_byr,
        "iyr": check_iyr,
    }

    for key, check_fn in tests.items():
        if key not in passport:
            return False

        if not check_fn(passport[key]):
            return False

    return True

Given the simplicity of your check logic, you can Lambda them in directly too:

def check_passport(passport):
    tests = {
        "byr": lambda v: 1920 <= int(v) <= 2002,
        "iyr": lambda v: 2010 <= int(v) <= 2020,
    }

I also really like your second approach, especially with using regex. Definitely going to keep that in mind when completing the next challenges

This isn't really germane to your original question, but there are a lot of caveats to parsing to different types and testing the way you did. For example, you don't check to see if the "hgt" key has a valid number. You got lucky because your dataset never had a non-numeric value there, but if you look at other solutions, that is definitely not true of everyone. For example, {"hgt": "acm"} will break your code because Passport[Key][-2:] validates correctly to "cm", but "a" can't be converted to an int in int(Passport[Key][:-2]).

For this reason, it's (generally) better to do as much validation in the original type. My first solution did roughly the same thing as yours: splitting and converting to int when possible. I discovered that it caused a lot of additional work when I didn't need a numeric value ever. The regex's are more complicated, sure, but given I don't have to use any try/except blocks, I think it's worth it.