2
\$\begingroup\$

I want to write a function to produce an inclusive range of Day such that Saturdays are excluded.

import Data.Time.Calendar
import Data.Time.Calendar.WeekDate

let sel3 (_, _, x) = x
let dateRange d1 d2 = takeWhile (<= d2) [ d | d <- map (flip addDays d1) [1..], (sel3 . toWeekDate) d /= 6]

I believe the code is correct as written, but I'd like to know if there's anything non-idiomatic.

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

You seem to be mixing compiled Haskell (import ...) with GHCi commands (top-level lets).

Top-level definitions should all be given type signatures, e.g.,

sel3 :: (a, b, c) -> c

Try to break down your functions into semantically interesting (and therefore nameable) chunks. For instance, (sel3 . toWeekDate) d /= 6 is a proposition on Days to ensure it isn't Saturday. So say that.

-- Usage: isSaturday (fromWeekDate 2016 06 22) == False
--        isSaturday (fromWeekDate 2016 06 25) == True
isSaturday :: Day -> Bool
isSaturday d = weekday d == 6
  where
    weekday :: Day -> Int
    weekday = sel3 . toWeekDate

Take advantage of available instances! Days are instances of Enum, so you can enumerate them directly without that messy map and takeWhile you've got going on.

rangeWithoutSaturdays :: Day -> Day -> [Day]
rangeWithoutSaturdays start end = filter (not . isSaturday) [start .. end]
\$\endgroup\$
  • \$\begingroup\$ Thanks for the comments. Would it be more appropriate to make isSaturday a where clause, since it's unlikely to be useful generally? \$\endgroup\$ – cloakedlearning Jun 22 '16 at 19:58
  • \$\begingroup\$ Maybe! A few things to keep in mind though—1) Deep nesting (wheres within wheres) starts to get pretty ugly and can make it difficult to follow flow in a function. 2) Testing. 3) Module export lists. 4) Documentation. I try to keep the number of tricky or non-obvious things in a function to one, like the magic number 6 meaning Saturday. Too many clever or non-obvious things in one place makes for difficult reading, and vertical space is cheap. \$\endgroup\$ – R B Jun 22 '16 at 21:40

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.