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\$
1
  • \$\begingroup\$ I believe your link should have been exercism.io but I can't make a one character edit. \$\endgroup\$
    – Matt
    Apr 14, 2017 at 18:42

3 Answers 3

6
\$\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\$
2
  • \$\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\$ Apr 16, 2017 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, 2017 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\$
5
  • 4
    \$\begingroup\$ In what way is scalability an issue for leap year determination? \$\endgroup\$ Apr 14, 2017 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, 2017 at 16:55
  • \$\begingroup\$ How is it more scalable? \$\endgroup\$
    – jpaugh
    Apr 14, 2017 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, 2017 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\$ Apr 16, 2017 at 17:59
0
\$\begingroup\$

I did another variation of the “list version” of my leap year check. I think it's more readable this time because it doesn't work with these (factor, boolean) pairs. The intend what is going one should be clearer, also because I think I did better naming:

module LeapYear (isLeapYear) where

isLeapYear :: Integer -> Bool
isLeapYear year = hasOddLength $ yearIsDivisibleByList
  where specialIntervals = [4, 100, 400]
        yearIsDivisibleByList = filter yearIsDivisibleBy $ specialIntervals
        yearIsDivisibleBy d = year `mod` d == 0
        hasOddLength = odd . length
```
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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