2
\$\begingroup\$

I need to define custom calendars and, in particular, test a DateTime for being a holiday. My current code is shown below. Is there a more concise/better way of doing this, preferably without sacrificing performance?

let private isHoliday2011 (dt:DateTime) =
  match dt.DayOfWeek with
  | DayOfWeek.Saturday | DayOfWeek.Sunday ->
    match (dt.Month, dt.Day) with
    | (3,5) -> false
    | _ -> true
  | _ ->
    match dt.Month with
    | 1 ->
      match dt.Day with
      | 3 | 4 | 5 | 6 | 7 | 10 -> true
      | _ -> false
    | 2 ->
      match dt.Day with
      | 23 -> true
      | _ -> false
    | 3 ->
      match dt.Day with
      | 7 | 8 -> true
      | _ -> false
    | 5 ->
      match dt.Day with
      | 2 | 9 -> true
      | _ -> false
    | 6 ->
      match dt.Day with
      | 13 -> true
      | _ -> false
    | 11 ->
      match dt.Day with
      | 4 -> true
      | _ -> false
    | _ -> false
\$\endgroup\$

migrated from stackoverflow.com Jul 27 '12 at 12:18

This question came from our site for professional and enthusiast programmers.

  • \$\begingroup\$ I think your code is as good as it gets. All the other answers involving data structures sacrifice performance for no real gain in clarity. To tweak your code for clarity, you could try merging match statements, this should not have any performance effects, e.g. gist.github.com/3189294 \$\endgroup\$ – t0yv0 Jul 27 '12 at 17:38
2
\$\begingroup\$

If you need to check for holidays repeatedly, I would go for Set and flatten pattern matching blocks for readability:

let holidays = set [ 1, 3; 1, 4; 1, 5; 1, 6; 1, 7; 1, 10 // month, day
                     2, 23
                     3, 7; 3, 8
                     5, 2; 5, 9
                     6, 13
                     11, 4
                    ]

let private isHoliday2011 (dt: DateTime) =
  match dt.DayOfWeek, dt.Month, dt.Day with
  | DayOfWeek.Saturday, m, d | DayOfWeek.Sunday, m, d -> m <> 3 || d <> 5    
  | _, m, d -> Set.contains (m, d) holidays
\$\endgroup\$
  • \$\begingroup\$ +1 DayOfWeek.Saturday, m, d | DayOfWeek.Sunday, m, d could be shortened (slightly) to (DayOfWeek.Saturday | DayOfWeek.Sunday), m, d. No need to bind m and d on both sides of |. \$\endgroup\$ – Daniel Jul 27 '12 at 16:14
2
\$\begingroup\$

I think the fastest, cleanest way you could check a large number of dates would be to memoize your date-checking function using a HashSet. Using HashSet<T> instead of the F# set is a better choice here because lookup is O(1) instead of O(log n) -- and you don't have to worry about mutability because the HashSet is never updated after it's created.

let isHoliday2011Memo =
    let holidays2011 =
        let jan1 = System.DateTime (2011, 1, 1)
        [| for i = 0 to 364 do
            yield jan1.AddDays <| float i |]
        |> Array.choose (fun day ->
            if isHoliday2011 day then
                Some day.DayOfYear
            else None)

    let holidayDaysOf2011 = System.Collections.Generic.HashSet<_> (holidays2011)

    fun (dt : DateTime) ->
        holidayDaysOf2011.Contains <| dt.DayOfYear

For even more speed, you could pre-compute the dates (using your function) -- and it means you don't need to include your date-computing function in your release code, because dates won't be calculated at run-time:

let isHoliday2011Precomputed =
    let holidayDaysOf2011 =
        System.Collections.Generic.HashSet<_> (
            [|1; 2; 3; 4; 5; 6; 7; 8; 9; 10; 15; 16; 22; 23; 29; 30; 36; 37; 43; 44; 50;
            51; 54; 57; 58; 65; 66; 67; 71; 72; 78; 79; 85; 86; 92; 93; 99; 100; 106;
            107; 113; 114; 120; 121; 122; 127; 128; 129; 134; 135; 141; 142; 148; 149;
            155; 156; 162; 163; 164; 169; 170; 176; 177; 183; 184; 190; 191; 197; 198;
            204; 205; 211; 212; 218; 219; 225; 226; 232; 233; 239; 240; 246; 247; 253;
            254; 260; 261; 267; 268; 274; 275; 281; 282; 288; 289; 295; 296; 302; 303;
            308; 309; 310; 316; 317; 323; 324; 330; 331; 337; 338; 344; 345; 351; 352; 358;
            359; 365; |])

    fun (dt : DateTime) ->
        holidayDaysOf2011.Contains <| dt.DayOfYear
\$\endgroup\$
1
\$\begingroup\$

I think using a list would be better - something like

let holidays = (1,3)::(1,4)::...
let isHoliday2011 (dt:DateTime) =
    let d,m = dt.Month, dt.Day
    holidays |> List.exists (fun (mm,dd) -> m=mm && d=dd)
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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