Skip to main content
2 of 2
Spelling and grammar
Reinderien
  • 71.1k
  • 5
  • 76
  • 256

Exception

If your algorithm cannot find a date, it is easier to raise an Exception than to return ''. Returning sentinel values instead of exceptions can lead to unexpected behaviour if the user of this function does not test for this sentinel value.

comments

Comments should explain why you did something, not how. # Take the last four digits tells you nothing more than the code itself. I would rather comment at field[-4 - i:n - i] why you did n - i instead of just -i.

nesting

Instead of nesting a number of if-clauses, it can be better to test the negative of the condition, and continue, so the rest of the code is less nested.

match

Don't test condition is True. Just do condition. In Python a lot of values can act as True or False in tests.

Your match is never used anyway; the moment you set it to True, you also return the result, so a while True: would have sufficed here.

field

This is a very unclear variable name. This method excepts a date in string format, so why not call the argument like that?

Return type

Your code does 2 things now. It looks for a date in the string, and converts that date to another format. It would be better to separate those 2 things, and return a datetime.datetime instead, and let the caller of this method worry about formatting that correctly.

while True

You use a while True-loop, with an incrementing counter. A better way to do this would be to either use for i in range(...) or using itertools.count: for i in itertools.count(). In this case you know there will be no more than len(field) - 7 iterations, so you might as well use that.

Revert the algorithm

You explicitly test whether the substring is 8 characters long, and then if it is in the right format. By changing the while True to the for-loop, you know the substring will be 8 characters long. Then it makes sense to first try to convert it to a datetime, and then check whether the year is correct:

def format_dates2(date_string):
    n = len(date_string)
    for i in range(n - 7):
        sub_string = date_string[-(8 + i) : n - i]
        # not just -i because that fails at i==0
        try:
            date = dt.strptime(sub_string, "%d%m%Y")
        except ValueError:
            continue
        if not (1919 <= date.year <= 2019):
            continue
        return date
    raise ValueError("Date not in the correct format")
Maarten Fabré
  • 9.4k
  • 1
  • 15
  • 27