HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

Getting the last date where a given week day occurred

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
lastthewheregettingdateweekdayoccurredgiven

Problem

I'm trying to learn a little bit about functional programming and as my tool I chose F# since I'm a .NET developer and the environment is more natural to me.

In one of my pet projects I'm dealing with dates, so I created a function to get the last date where a given week day occurred. For example, last Tuesday was yesterday (DateTime.Now.AddDays(-1.0)).

#light

module DateTimeExtensions
    open System    

    let rec whenWasLastBasedOnDate (weekDay:DayOfWeek, currentDate:DateTime) = 
        match currentDate with
        | _ when currentDate.DayOfWeek = weekDay -> currentDate
        | _ -> whenWasLastBasedOnDate(weekDay, currentDate.AddDays(-1.0))

    let whenWasLast (weekDay:DayOfWeek) = 
        let currentDate = DateTime.Now
        match currentDate with
        | _ when currentDate.DayOfWeek = weekDay -> currentDate
        | _ -> whenWasLastBasedOnDate(weekDay, currentDate.AddDays(-1.0))    

    let now = DateTime.Now
    let lastSunday = whenWasLast DayOfWeek.Sunday
    let lastMonday = whenWasLast DayOfWeek.Monday
    let lastTuesday = whenWasLast DayOfWeek.Tuesday
    let lastWednesday = whenWasLast DayOfWeek.Wednesday
    let lastThursday = whenWasLast DayOfWeek.Thursday
    let lastFriday = whenWasLast DayOfWeek.Friday
    let lastSaturday = whenWasLast DayOfWeek.Saturday

Solution

The most obvious point to make is that whenWasLast repeats the entire code of whenWasLastBasedOnDate. You can (and should) simply write the whole method by just calling whenWasLastBasedOnDate:

let whenWasLast (weekDay:DayOfWeek) =
    whenWasLastBasedOnDate(weekDay, DateTime.Now)


Another point is that you should write AddDays(-1.0) as AddDays -1.0. In languages with ML-like syntax it is generally discouraged to add redundant parentheses around function arguments because it encourages the misconception that the parentheses are part of the method call syntax.

On a somewhat more subjective note, I think that using if would read nicer than a match without any patterns.

On a more general design note it is generally considered good style in functional programming to use higher order functions to express the code more abstractly. For example instead of saying "Check if the given day was a weekDay. If so, return it, if not repeat with the day before" you could say "Look at the last seven days and return the one who is a weekDay". This is not only more idiomatic functional programming, but also makes it impossible to get into infinite recursion by getting the exit condition wrong. In code it would look like this:

[-6.0..0.0] |> Seq.map currentDate.AddDays |>
    Seq.find (fun d -> d.DayOfWeek = weekDay)

Code Snippets

let whenWasLast (weekDay:DayOfWeek) =
    whenWasLastBasedOnDate(weekDay, DateTime.Now)
[-6.0..0.0] |> Seq.map currentDate.AddDays |>
    Seq.find (fun d -> d.DayOfWeek = weekDay)

Context

StackExchange Code Review Q#572, answer score: 11

Revisions (0)

No revisions yet.