I am getting used to how things should be written/thought about in (idiomatic) Haskell. I wrote a FizzBuzz program that prints to the console. I wanted to generate things from the core info of the divisors and the strings that go with them, and an array of such rules to be processed. This is in some sense a port of my original Purescript question here (deleted), but I'm interested in Haskell conventions independently, especially when they might differ from Purescript.
All improvements are welcome, but I am focusing primarily on "is this a reasonable way a user of Haskell might organize things?". One thing I'm looking to understand is when/why I might prefer making something like Fizzrule
a type
or newtype
of (Int, String)
instead of the brand new datatype here. And honestly, mapMaybe
is just a carryover from how I'm used to writing things in modern Wolfram Language, so I'm curious if it's the right thing here (maybe it's a special case of something more general I should be thinking about?). Finally, is mapM_
the most natural thing to use for a task like this?
module Main where
import Data.Foldable(fold)
import Data.List(null)
import Data.Maybe(mapMaybe)
data Fizzrule = Fizzrule {divisor :: Int, yell :: String}
rule3 :: Fizzrule
rule3 = Fizzrule 3 "Fizz"
rule5 :: Fizzrule
rule5 = Fizzrule 5 "Buzz"
rule7 :: Fizzrule
rule7 = Fizzrule 7 "Quux"
processrule :: Int -> Fizzrule -> Maybe String
processrule int rule = if int `mod` (divisor rule) == 0
then Just (yell rule) else Nothing
fizzbuzz :: [Fizzrule] -> Int -> String
fizzbuzz rules int = if null strings then show int else fold strings
where strings = mapMaybe (processrule int) rules
main :: IO()
main = mapM_ (putStrLn . fizzbuzz [rule3, rule5, rule7]) [1..100]
1 Answer 1
It would be more usual to replace the if null strings...
conditional with a case
, and concat
is more commonly used than fold
for lists:
fizzbuzz :: [Fizzrule] -> Int -> String
fizzbuzz rules int
= case mapMaybe (processrule int) rules of
[] -> show int
strings -> concat strings
Similarly, processrule
would more commonly be written with guards than an if-then-else
:
processrule :: Int -> Fizzrule -> Maybe String
processrule int rule
| int `mod` (divisor rule) == 0 = Just (yell rule)
| otherwise = Nothing
For simple structures like Fizzrule
, it's pretty common to dispense with field selectors and pattern match directly, so this would probably be further rewritten. Also, it's more usual to use single character arguments like n
in preference to int
, though str
and lst
are often used for strings and lists:
processrule :: Int -> Fizzrule -> Maybe String
processrule n (Fizzrule d str)
| n `mod` d == 0 = Just str
| otherwise = Nothing
Then the mapMaybe
and processRule
functions aren't actually doing much, so it would be more usual to replace them with a list comprehension:
fizzbuzz :: [Fizzrule] -> Int -> String
fizzbuzz rules n
= case [s | Fizzrule d s <- rules, d `divides` n] of
[] -> show n
strings -> concat strings
where d `divides` m = m `rem` d == 0
Arguably, it might be more usual to keep the "map" in main
pure and output all the lines at once. (Even when run as a pure computation, it's all done lazily, so no worry that you need to wait for the computation to complete if you try to FizzBuzz the first billion integers or even run it on an infinite list [1..]
). Also, note the type signature spacing -- it's always IO ()
and never IO()
.
main :: IO ()
main = putStr . unlines $ map (fizzbuzz rules) [1..100]
Naming all the rules seems like overkill, and Fizzrule
takes forever to type, so maybe:
{-# OPTIONS_GHC -Wall #-}
module Main where
data Rule = Rule Int String
myrules :: [Rule]
myrules =
[ Rule 3 "Fizz"
, Rule 5 "Buzz"
, Rule 7 "Quux"
]
fizzbuzz :: [Rule] -> Int -> String
fizzbuzz rules n
= case [s | Rule d s <- rules, d `divides` n] of
[] -> show n
strings -> concat strings
where d `divides` m = m `rem` d == 0
main :: IO ()
main = putStr . unlines $ map (fizzbuzz myrules) [1..100]
That looks more idiomatic to me.
Writing the Rule
as a newtype:
newtype Rule = Rule (Int, String)
would be silly. The main point of newtype
is to introduce a type and associated constructor that's "free" from a performance standpoint because it's erased at compilation time (while also allowing for certain conveniences related to automatically deriving type class instances). However, in:
newtype Rule = Rule (Int, String)
the Rule
constructor can be erased, but the pair constructor (normally written (,)
) is still there. If you just write a data
type in the first place:
data Rule = Rule Int String
then that replaces the (,)
constructor with the Rule
constructor more explicitly. There's no advantage here to newtype Rule
-- it just requires extra typing when constructing or pattern matching values.
The choice between data Rule = ...
and type Rule = (Int, String)
is more a matter of personal taste. I'd probably go with the data Rule
type.
To summarize some of the principles used above:
if-then-else
statements are rare in Haskell and are usually replaced bycase
constructs (or the implicitcase
construct of function definitions), possibly with the help of guards. It's especially rare to seeif null lst then...
since this is almost always better written as acase
.- Haskellers usually inline helper functions like
processrule
that are only used in one place and have no general applicability. In constrast, Haskellers often define trivial functions likedivides
inwhere
clauses where they improve readability. I'm arguing here that the list comprehension is more directly readable than the function nameprocessrule
, butdivides
is more readable than them `rem` d == 0
expression. It's a judgement call. - At least for "small" data structures, pattern matching is usually used in preference to field selectors.
- Pure code is often used in preference to impure code, including where you have an action where additional purity can be factored out (e.g., instead of
mapM_
over multiple IO actions,map
over pure values and pass that to a single IO action).
-
\$\begingroup\$ Thank for this in-depth answer! I see now that list comprehensions with conditions are better than shoehorning in
Maybe
where I don't have a semantic use for it. And the reminders aboutcase
and maximizing pure code make total sense. It may take me some time to internalize how to handle things likeprocessrule
anddivides
, but it's clear your rewrite is more readable. \$\endgroup\$Mark S.– Mark S.2019年11月07日 13:24:16 +00:00Commented Nov 7, 2019 at 13:24