5
\$\begingroup\$

When fridays_135 is called, a list of dates between the start date and end date is produces by date_range. That list is then narrowed down to just the Friday dates and then grouped by month by fridays_by_month. Finally, fridays_135 loops through those groups of months, extracts the 0th, 2nd, and 4th element in that list, and then appends them to the fri_135 list to produce a list of dates corresponding to the 1st, 3rd, and 5th Friday of ever month within the original date range, ordered by year and then by month.

from datetime import date, timedelta

start_date = date(2021, 1, 1)
end_date = date(2025, 12, 31)

''' Returns a list of dates between two dates (inclusive)'''
def date_range(start = start_date, end = end_date):
    delta = end - start
    dates = [start + timedelta(days=i) for i in range(delta.days + 1)]
    return dates
    
''' Returns a list of lists containing the Friday dates for each month in a given range'''
def fridays_by_month():
    year = range(1, 13)
    years = range(start_date.year, end_date.year + 1)
    date_range_fris = [date for date in date_range() if date.strftime('%a') == "Fri"]
    range_fris = []
    for yr in years:
        for month in year:
            month_fris = []
            for date in date_range_fris:
                if date.strftime('%m') == str(f'{month:02}') and date.year == yr:
                    month_fris.append(date)
            range_fris.append(month_fris)
            month_fris = []
    return range_fris
    
'''Returns a list of first, third, and fifth Fridays of each month in a given range of years, as a string, ordered by year and then by month'''
def fridays_135():
    fri_ordinals = [0, 2, 4]
    month_fris = fridays_by_month()
    fri_135 = []
    for month in month_fris:
        for index, date in enumerate(month):
            if index in fri_ordinals:
                fri_135.append(str(date))
    return fri_135

print(fridays_135())
\$\endgroup\$
3
  • \$\begingroup\$ You shouldn't use comments to describe your function, but docstrings. \$\endgroup\$
    – Kyrela
    Dec 3, 2021 at 9:21
  • \$\begingroup\$ I have rolled back Rev 5 → 3. Please see What to do when someone answers. \$\endgroup\$ Dec 3, 2021 at 19:08
  • \$\begingroup\$ For start_date = date(2021, 1, 8), the first date you produce is '2021-01-08'. Is that actually correct? That's the month's second Friday... \$\endgroup\$ Dec 3, 2021 at 22:18

3 Answers 3

11
\$\begingroup\$

It seems rather inefficient to generate every date, then filter out non-Fridays, then filter out ones that arent't the first, third, or fifth Friday. Calculate directly them instead.

Find the first Friday in the range:

friday = start + timedelta((FRIDAY - start.weekday())%7)

Move forward a week if it's not the 1st, 3rd, or 5th Friday:

ONE_WEEK = timedelta(weeks=1)

week = (friday.day - 1)//7 + 1
if week not in (1,3,5):
    friday += ONE_WEEK

While friday is still in the data range, yield the date. If it's the 1st or 3d Friday, add one week, otherwise add two weeks (day 22 is the start of the 4th week):

TWO_WEEKS = timedelta(weeks=2)

while friday < end:
    yield friday
    
    if friday.day < 22:
        friday += TWO_WEEKS
    else:
        friday += ONE_WEEK

Putting it all together:

# date.weekday() returns 0 for Monday, so 4 = Friday
FRIDAY = 4

ONE_WEEK = timedelta(weeks=1)
TWO_WEEKS = timedelta(weeks=2)

def gen_friday_135(start, end):
    friday = start + timedelta((FRIDAY - start.weekday())%7)
    
    week = (friday.day - 1)//7 + 1
    if week not in (1,3,5):
        friday += ONE_WEEK

    while friday < end:
        yield friday
        
        if friday.day < 22:
            friday += TWO_WEEKS
        else:
            friday += ONE_WEEK
\$\endgroup\$
5
  • \$\begingroup\$ Wow, almost all of my code is is unnecessary! \$\endgroup\$ Dec 2, 2021 at 22:17
  • \$\begingroup\$ What is the purpose of the minus 1 in the week variable? I tried the generator with and without that minus 1, and it produced the same list in both cases \$\endgroup\$ Dec 3, 2021 at 18:15
  • 1
    \$\begingroup\$ @StephenWilliams, friday.day returns integers in the range 1-31. The minus one converts it so the range starts at zero so the // operator gives the correct week number. Without it, the formula would calculate the 7th as being in the second week. \$\endgroup\$
    – RootTwo
    Dec 3, 2021 at 23:39
  • \$\begingroup\$ That makes sense to me. I'm still wondering why its removal didn't affect the list, though. \$\endgroup\$ Dec 4, 2021 at 2:56
  • 1
    \$\begingroup\$ @StephenWilliams, without the -1 gen_friday_135(start=date(2021, 5, 2), end=date(2021, 5, 30)) will yield the 5/14 and 5/28 (the 2nd and 4th Fridays) instead of 5/7 and 5/21 (the 1st and 3rd Fridays). \$\endgroup\$
    – RootTwo
    Dec 4, 2021 at 4:24
5
\$\begingroup\$

Here are a few pointers on how your code can be improved:

Style

PEP8 recommends surrounding top-level functions by two blank lines. This improves readability somewhat.

Also, docstrings are supposed to come after the function signature, not before, as specified in PEP257. This is required for the docstring to be handled by Docutils and such software.

Other than that, your style and layout is good.

Globals

You handle the start and end date with global variables. This is bad practice for a number of reason, including code readability (the code flow is harder to follow if you have to find global declarations, especially if your code gets longer), reusability, maintainability (it's hard to keep track which part of the code act on constants).

You should pass these values as function arguments

Name shadowing

date is a class name that you explicitly import, yet you overwrite it in some places, such as date_range_fris = [date for date in date_range() [...]].

Although there is not much damage as the scope is limited, I find this confusing to read. You should pick a different variable name.

Casting values to string

When checking if a given day is a friday, you compare the string representation of the date to a hard-coded string.

This is wasteful as it requires extra work to get the string from a date and to compare strings.

It would be better to use the weekday() method of date instances, which returns an integer. To keep a high readability, compare the value with a name constant.

FRIDAY = 4

# [...]

    date_range_fris = [d for d in date_range() if d.weekday() == FRIDAY]

Same remark for month comparison.

\$\endgroup\$
5
\$\begingroup\$

You are organizing your code in functions, which is good; however, you aren't taking full advantage of their benefits because most of your functions operate on the basis of global variables rather than function arguments. A simple data-oriented program like this has no need at all for global variables. In such a context, every function should take input arguments and return the corresponding output data. That's a key principle of effective software engineering, and you should embrace it fully whenever practical.

You have a useful utility in date_range(). One can adjust it a bit to remove its hard-coded attachment to global variables and to convert it to a generator function (not required, but something that makes some sense given the other functions I have in mind; I think on my first draft of this answer I misunderstood your purpose).

from datetime import timedelta, date

def date_range(start, end):
    for i in range((end - start).days + 1):
        yield start + timedelta(days = i)

From that foundation, one can select the Fridays:

def fridays_in_range(start, end):
    for d in date_range(start, end):
        if d.weekday() == 4:
            yield d

And then it's easy to do the grouping and selecting via some handy utilities from itertools.

from operator import attrgetter
from itertools import groupby, islice

def fridays_135(start, end):
    for k, group in groupby(fridays_in_range(start, end), attrgetter('month')):
        for f in islice(group, 0, None, 2):
            yield f

One could write all of that in a single function, I suppose, perhaps even in fewer lines of code. But I think I prefer this approach because it focuses the mind on building simple, useful functions – one on top of the other. Notice also that in this particular case, the code contains very little of our own algorithmic machinations: the grubby details (even selecting 1st, 3rd, and 5th Fridays) are farmed out to various parts of the Python standard library.

\$\endgroup\$
4
  • \$\begingroup\$ Wow, I really am over complicating it! Thank you. \$\endgroup\$ Dec 2, 2021 at 17:36
  • 1
    \$\begingroup\$ date_range imo should be a generator :) \$\endgroup\$
    – hjpotter92
    Dec 2, 2021 at 18:00
  • \$\begingroup\$ @FMc When you say that my method for finding Fridays is overly complex, do you mean that one line of code or the entire fridays_by_month function? The bulk of the work done in that function is the grouping of the dates into months that can then be checked for positional status. \$\endgroup\$ Dec 2, 2021 at 18:32
  • \$\begingroup\$ @StephenWilliams I think I misunderstood your question before. I augmented my response. \$\endgroup\$
    – FMc
    Dec 2, 2021 at 23:46

Your Answer

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

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