4
\$\begingroup\$

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]
\$\endgroup\$
3
\$\begingroup\$

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 by case constructs (or the implicit case construct of function definitions), possibly with the help of guards. It's especially rare to see if null lst then... since this is almost always better written as a case.
  • 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 like divides in where clauses where they improve readability. I'm arguing here that the list comprehension is more directly readable than the function name processrule, but divides is more readable than the m `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).
\$\endgroup\$
  • \$\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 about case and maximizing pure code make total sense. It may take me some time to internalize how to handle things like processrule and divides, but it's clear your rewrite is more readable. \$\endgroup\$ – Mark S. Nov 7 at 13:24

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.