# Exception If your algorithm can not find a date, it is easier to raise an Exception then 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 Commenst 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 then `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")