4
\$\begingroup\$

One of the first Haskell puzzles on http://exercism.io/ is to implement a leap year check. I've done this twice.

Using pattern matching:

isLeapYear :: Integer -> Bool
isLeapYear year
  | year `mod` 400 == 0 = True
  | year `mod` 100 == 0 = False
  | year `mod` 4 == 0   = True
  | otherwise           = False

Using the bind operator >>=:

isLeapYear :: Integer -> Bool
isLeapYear year = head $
                  [(400, True), (100, False), (4, True), (1, False)]
                  >>= check year
  where check y (interval, isLeap) = [isLeap | y `mod` interval == 0]

I'd like to know which implementation is “better“ / more idiomatic in Haskell. I am unsure whether I might have misused a t0o powerful concept in the second try, and the first try might just be more readable.

\$\endgroup\$
  • \$\begingroup\$ I believe your link should have been exercism.io but I can't make a one character edit. \$\endgroup\$ – Matt Apr 14 '17 at 18:42
5
\$\begingroup\$

The first one is a lot more readable, whereas the second one uses a "hack". I would go with the first one, except that I would use rem, which is a little bit faster. And one could introduce some DRY:

isDivisibleBy :: Integral n => n -> n -> Bool
isDivisibleBy x n = x `rem` n == 0

isLeapYear :: Integer -> Bool
isLeapYear year
  | divBy 400 = True
  | divBy 100 = False
  | divBy   4 = True
  | otherwise = False
 where
   divBy n = year `isDivisibleBy` n

That being said, for a programming challenge, your version is perfectly fine:

isLeapYear :: Integer -> Bool
isLeapYear year
  | year `rem` 400 == 0 = True
  | year `rem` 100 == 0 = False
  | year `rem`   4 == 0 = True
  | otherwise           = False

The latter can be rewritten without >>= as list comprehension:

isLeapYear :: Integer -> Bool
isLeapYear year = head [isLeap | (interval, isLeap) <- classifications
                               , year `isDivisibleBy` interval]
  where
    classifications = [(400, True), (100, False), (4, True), (1, False)]

You could get rid of the "hack" with safeHead and maybe False, but that's left as an exercise.

If you really want to use check, remove the y. It just introduces an additional error source:

isLeapYear :: Integer -> Bool
isLeapYear year = head $
                  [(400, True), (100, False), (4, True), (1, False)]
                  >>= check
  where check (interval, isLeap) = [isLeap | year `rem` interval == 0]

Note that this shows perfectly that >>= is just flip concatMap for lists. So let's take advantage:

isLeapYear :: Integer -> Bool
isLeapYear year = head $ concatMap check classifications ++ [False]
  where 
    check (interval, isLeap) = [isLeap | year `rem` interval == 0]
    classifications = [(400, True), (100, False), (4, True)]

Which is easier to grasp than the version with >>=.

\$\endgroup\$
  • \$\begingroup\$ Thanks for the very detailed comment. I think the second version with your improvements is very interesting as a pure exercise. But in production code, the first style might really be better. \$\endgroup\$ – Matthias Wimmer Apr 16 '17 at 18:03
  • \$\begingroup\$ @MatthiasWimmer I forgot to add the difference between both: the latter gives rise to isX :: [(Integer, Bool)] -> Integer -> Bool, and isLeapYear = isX classifications, aka you could change the list later on. That's not possible with the guard variant. Gurkenglas answer contains more details. As I said in my comment: a nice generic way would be lookupBy :: (k -> Bool) -> [(k,v)] -> Maybe v, which is a great exercise by the way (although already somewhat solved by Gurkenglas). \$\endgroup\$ – Zeta Apr 16 '17 at 19:37
2
\$\begingroup\$

In general, I'd go for the second sort of approach. It scales better. I'd eliminate four names and the hack like so, at the cost of fst and snd:

isLeapYear :: Integer -> Bool
isLeapYear year = maybe False snd
  $ find ((==0) . mod year . fst)
  [(400, True), (100, False), (4, True)]
\$\endgroup\$
  • 4
    \$\begingroup\$ In what way is scalability an issue for leap year determination? \$\endgroup\$ – 200_success Apr 14 '17 at 15:51
  • \$\begingroup\$ Obviously it's not. But not everyone can write the linux kernel from scratch, so sometimes you have to find artificial problems to practice scalable techniques. \$\endgroup\$ – amalloy Apr 14 '17 at 16:55
  • \$\begingroup\$ How is it more scalable? \$\endgroup\$ – jpaugh Apr 14 '17 at 17:26
  • \$\begingroup\$ @jpaugh it contains a way to write lookupBy :: (k -> Bool) -> [(k, v)] -> Maybe v, which is a nice little exercise and a great generalization of lookup. But to be honest, I think that gets lost and is not that apparent. \$\endgroup\$ – Zeta Apr 14 '17 at 17:30
  • \$\begingroup\$ Thanks for your comment. I feel bad not being able to “accept” both comments, as they helped me both. It's not like a bug fix, where there is one answer that solves the problem first. \$\endgroup\$ – Matthias Wimmer Apr 16 '17 at 17:59

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.