Skip to content

Code Quality Issue #2

@bartmika

Description

@bartmika

The following comment was made by ppetreus via this reddit post.

Before using an external package, I perform a code quality audit.
I reviewed this function for code quality:

// FirstDayOfLastISOWeek returns the previous week's monday date.
func FirstDayOfLastISOWeek(now func() time.Time) time.Time {
    dt := now()

    // iterate back to Monday
    for dt.Weekday() != time.Monday {
        dt = dt.AddDate(0, 0, -1)
    }
    dt = dt.AddDate(0, 0, -1) // Skip the current monday we are on!

    // iterate ONCE AGAIN back to Monday
    for dt.Weekday() != time.Monday {
        dt = dt.AddDate(0, 0, -1)
    }

    return dt
}

Algorithms are important.
You use expensive iteration twice. Also, you begin the second iteration on the last day (Sunday) of the previous week. Why iterate a constant number of times (6) for the first day (Monday) of the previous week?

A simple integer arithmetic computation and date normalization will do.
package time
The month, day, hour, min, sec, and nsec values may be outside their usual ranges and will be normalized during the conversion. For example, October 32 converts to November 1.

// FirstDayOfLastISOWeek returns the previous week's Monday date.
func FirstDayOfLastISOWeek(now func() time.Time) time.Time {
    dt := now()
    // the number of days from this week's Monday
    thisMonday := (int(dt.Weekday()) + 6) % 7
    return dt.AddDate(0, 0, -thisMonday-7)
}

Your spelling of Monday is inconsistent: monday and Monday.

// FirstDayOfLastISOWeek returns the previous week's monday date.

// iterate back to Monday

// Skip the current monday we are on!

// iterate ONCE AGAIN back to Monday

Others in this thread have pointed out obvious misspellings:

func GetForstDateFromMonthAndYear
You are SHOUTING at us:
// iterate ONCE AGAIN back to Monday

Metadata

Metadata

Assignees

Labels

enhancementNew feature or request

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions