1
\$\begingroup\$

I have been working on porting my PowerShell 7 scripts to Python 3.9.5 lately, re-implementing the same logic in a different programming language, keeping what makes it work and removing unnecessary parts, improving the code, and this is one of the scripts that has been re-implemented.

The code is simple, and I know this has been implemented countless times, but it does get the job done without using the builtin datetime module, and it accepts two strings representing dates in nine date formats and returns the difference between the two dates:

'yyyy-MM-dd'
'yyyy/MM/dd'
'MM/dd/yyyy'
'MMM dd, yyyy'
'dd MMM, yyyy'
'MMMM dd, yyyy'
'dd MMMM, yyyy'
'yyyy, MMM dd'
'yyyy, MMMM dd'

And it is completely working.

So here is the code:

import re
import sys

Months = (
    {'Month':'January', 'Days':31},
    {'Month':'February', 'Days':28},
    {'Month':'March', 'Days':31},
    {'Month':'April', 'Days':30},
    {'Month':'May', 'Days':31},
    {'Month':'June', 'Days':30},
    {'Month':'July', 'Days':31},
    {'Month':'August', 'Days':31},
    {'Month':'September', 'Days':30},
    {'Month':'October', 'Days':31},
    {'Month':'November', 'Days':30},
    {'Month':'December', 'Days':31}
)

Culture = (
    {'Format':'yyyy-MM-dd', 'Regex':'^\d{4}\-0?([2-9]|1[0-2]?)\-(0?(3[01]|[12][0-9]|[1-9]))$'},
    {'Format':'yyyy/MM/dd', 'Regex':'^\d{4}\/0?([2-9]|1[0-2]?)\/(0?(3[01]|[12][0-9]|[1-9]))$'},
    {'Format':'MM/dd/yyyy', 'Regex':'^0?([2-9]|1[0-2]?)\/(0?(3[01]|[12][0-9]|[1-9]))\/\d{4}$'},
    {'Format':'MMM dd, yyyy', 'Regex':'^[A-Za-z]{3} (0?(3[01]|[12][0-9]|[1-9])), \d{4}$'},
    {'Format':'dd MMM, yyyy', 'Regex':'^(0?(3[01]|[12][0-9]|[1-9])) [A-Za-z]{3}, \d{4}$'},
    {'Format':'MMMM dd, yyyy', 'Regex':'^[A-Za-z]{3,9} (0?(3[01]|[12][0-9]|[1-9])), \d{4}$'},
    {'Format':'dd MMMM, yyyy', 'Regex':'^(0?(3[01]|[12][0-9]|[1-9])) [A-Za-z]{3,9}, \d{4}$'},
    {'Format':'yyyy, MMM dd', 'Regex':'^\d{4}, [A-Za-z]{3} (0?(3[01]|[12][0-9]|[1-9]))$'},
    {'Format':'yyyy, MMMM dd', 'Regex':'^\d{4}, [A-Za-z]{3,9} (0?(3[01]|[12][0-9]|[1-9]))$'}
)

def splitter(date, x, y):
    date = date.replace(',','').split(x)
    y = list(map(int, y.split(',')))
    year = int(date[y[0]])
    month = date[y[1]]
    if not re.match('^\d+$', month):
        for i in Months:
            if re.match(f'{month}',i['Month']): month = Months.index(i) + 1
    month = int(month)
    day = int(date[y[2]])
    return {'Year': year, 'Month': month, 'Day': day}

def parsedate(date):
    for a in range(len(Culture)):
        if re.match('%s' % Culture[a]['Regex'], date): i = a; break
    if i == 0: return splitter(date, '-', '0,1,2')
    elif i == 1: return splitter(date, '/', '0,1,2')
    elif i == 2: return splitter(date, '/', '2,0,1')
    elif i == 3: return splitter(date, ' ', '2,0,1')
    elif i == 4: return splitter(date, ' ', '2,1,0')
    elif i == 5: return splitter(date, ' ', '2,0,1')
    elif i == 6: return splitter(date, ' ', '2,1,0')
    elif i == 7: return splitter(date, ' ', '0,1,2')
    elif i == 8: return splitter(date, ' ', '0,1,2')

def totaldays(date):
    ye = date['Year']
    y = ye
    mon = date['Month']
    d = date['Day']
    if mon <= 2: y -= 1
    leaps = y // 4 - y // 100 + y // 400
    m = 0
    for b in range(mon - 1): m += int(Months[b].get('Days'))
    days = (ye - 1) * 365 + m + d + leaps
    return days

def diffdate(start, end):
    date1 = totaldays(parsedate(start))
    date2 = totaldays(parsedate(end))
    ddate = date2 - date1
    print(ddate)

start = sys.argv[1]
end = sys.argv[2]
diffdate(start, end)

As usual, I wonder how it can be improved, there may be multiple areas that can be improved but now I mostly care about the switch replacement and how to get parameters from command line, the 8 elif statements do the job but it makes the code look awful, I have tried to use a dictionary but it throws date is undefined error, and I don't want to import sys just to get the arguments from command line, I don't know if there is an automatic variable just like PowerShell's $Args.

Please help me improve my code, you can share whatever you think on my code.

\$\endgroup\$
1

2 Answers 2

4
\$\begingroup\$

Modern Python has so many conveniences for creating simple data objects (namedtuple, dataclass, and the excellent attrs library), that it's usually a mistake to structure your internal data using ordinary dicts, lists, or tuples. These simple data objects not only clean up the code from a readability perspective (as shown in multiple places below) but also encourage you to focus your attention where it should be: defining the meaningful data entities to facilitate and support the algorithmic needs of the program. Organize the data the right way, and the algorithms will usually flow naturally.

Case in point: you don't need an 8-way switch; you need richer data. The splitter() arguments belong in the realm of data (Culture instances in the code below), not algorithm.

Another case in point: don't drive your algorithm with data that requires parsing. For example, if you think you need a tuple of indexes like (2, 0, 1), don't store it in a comma-delimited string and complicate your algorithmic code with the unpacking logic. Just store the indexes directly as a tuple of integers. The core problem in software writing is reducing and managing complexity, and the greatest complexity is always algorithmic. Investing more in creating "smarter" or "richer" data will help you keep the algorithmic parts of the code easier to understand and maintain.

Python has a built-in sum() function.

When possible (which means almost always) iterate directly over a collection (ie, list, tuple) rather than over the indexes. If you need the indexes too, use enumerate().

Abandon your style of trying to cram extra logic on each line:

if foo > 1233: x = 12; y = 34

I went through that phase for a brief time early in my Python journey. It doesn't scale well and it's harder to maintain over the long term on a project. Again, complexity is the biggest foe in software engineering: one way to reducing complexity slightly (and increase readability) is to favor shorter lines of code over longer ones. Within reason, taller/skinnier texts are easier on readers than shorter/wider texts (the publishing industry has understood this for decades), so if you are cramming more code into single lines, you are economizing on the wrong thing (ie, saving lines while spending readability).

Similarly, abandon your aversion to importing parts of the Python standard library. It's fine to avoid those things in the name of learning. But that's the only good reason I know of. So get command-line arguments from sys.argv guilt-free. It's the way to go!

After the simplification from those changes are made, the splitter() function seems unnecessary. Instead, I would suggest a different helper function just to parse the month. And that logic doesn't require regex: you can use simpler things like isdigit() and startswith(). Also, a Month should know its number. Again, use richer data to simplify algorithm.

Other improvements for you to consider:

  • If you invest more in your regular expressions so that they use named captures, you can drop the tedious details in Culture.separator and Culture.indexes. Instead, you can grab the year, month, and day directly from the re.Match object, by name. Yet again, smart data, simple code.

  • When given invalid input, your code just blows up. Give some thought to whether you want to invest in better error handling/reporting.

  • Dates are notoriously complicated. There are probably weird edge-case bugs in your code. Are you sure you don't want to use datetime?


import re
import sys
from collections import namedtuple

# Simple data objects are handy as hell. Use more of them.

Month = namedtuple('Month', 'name number days')
Culture = namedtuple('Culture', 'format separator indexes regex')
Date = namedtuple('Date', 'year month day')

Months = (
    Month('January', 1, 31),
    Month('February', 2, 28),
    Month('March', 3, 31),
    Month('April', 4, 30),
    Month('May', 5, 31),
    Month('June', 6, 30),
    Month('July', 7, 31),
    Month('August', 8, 31),
    Month('September', 9, 30),
    Month('October', 10, 31),
    Month('November', 11, 30),
    Month('December', 12, 31),
)

Cultures = (
    Culture('yyyy-MM-dd',    '-', (0,1,2), '^\d{4}\-0?([2-9]|1[0-2]?)\-(0?(3[01]|[12][0-9]|[1-9]))$'),
    Culture('yyyy/MM/dd',    '/', (0,1,2), '^\d{4}\/0?([2-9]|1[0-2]?)\/(0?(3[01]|[12][0-9]|[1-9]))$'),
    Culture('MM/dd/yyyy',    '/', (2,0,1), '^0?([2-9]|1[0-2]?)\/(0?(3[01]|[12][0-9]|[1-9]))\/\d{4}$'),
    Culture('MMM dd, yyyy',  ' ', (2,0,1), '^[A-Za-z]{3} (0?(3[01]|[12][0-9]|[1-9])), \d{4}$'),
    Culture('dd MMM, yyyy',  ' ', (2,1,0), '^(0?(3[01]|[12][0-9]|[1-9])) [A-Za-z]{3}, \d{4}$'),
    Culture('MMMM dd, yyyy', ' ', (2,0,1), '^[A-Za-z]{3,9} (0?(3[01]|[12][0-9]|[1-9])), \d{4}$'),
    Culture('dd MMMM, yyyy', ' ', (2,1,0), '^(0?(3[01]|[12][0-9]|[1-9])) [A-Za-z]{3,9}, \d{4}$'),
    Culture('yyyy, MMM dd',  ' ', (0,1,2), '^\d{4}, [A-Za-z]{3} (0?(3[01]|[12][0-9]|[1-9]))$'),
    Culture('yyyy, MMMM dd', ' ', (0,1,2), '^\d{4}, [A-Za-z]{3,9} (0?(3[01]|[12][0-9]|[1-9]))$'),
)

# A purely stylistic point: organize modules the way you want to read them.
# Most humans like to read things top to bottom.

def main(args):
    start, end = args
    ddate = diffdate(start, end)
    print(ddate)

def diffdate(start, end):
    d1 = totaldays(parsedate(start))
    d2 = totaldays(parsedate(end))
    return d2 - d1

def totaldays(date):
    y = date.year
    if date.month <= 2:
        y -= 1
    leaps = y // 4 - y // 100 + y // 400
    m = sum(Months[b].days for b in range(date.month - 1))
    days = (date.year - 1) * 365 + m + date.day + leaps
    return days

def parsedate(date):
    for c in Cultures:
        if re.match(c.regex, date):
            parts = date.replace(',', '').split(c.separator)
            return Date(
                int(parts[c.indexes[0]]),
                parsemonth(parts[c.indexes[1]]),
                int(parts[c.indexes[2]]),
            )
    # Raise an error if we get here.

def parsemonth(month):
    if month.isdigit():
        return int(month)
    else:
        for m in Months:
            if m.name.lower().startswith(month.lower()):
                return m.number
    # Raise an error if we get here.

if __name__ == '__main__':
    main(sys.argv[1:])

\$\endgroup\$
2
\$\begingroup\$

This is a good approach you have done if you have to skip datetime module.

There are many more cultures that you have not added but that's different topic.

First, I will answer your question related to command line argument. NO there is no clean/good solution for that.

Second, I will only review the main issue, replacing if-else ladder with switch.

You have already done the ground work for me. :)

Decider = [
  ('-', '0,1,2'), ('/', '0,1,2'), ('/', '2,0,1'),
  (' ', '2,0,1'), (' ', '2,1,0'), (' ', '2,0,1'),
  (' ', '2,1,0'), (' ', '0,1,2'), (' ', '0,1,2')
]

# use like
splitter(date, *Decider[i])

Hope this helps.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.