# How can I improve this Haskell code?

This code solves the problem of FizzBuzz. Is it possible in any way to improve it?

main = main' 1 where
main' n = do
(putStrLn . choose) (show n, "Fizz", "Buzz", "FizzBuzz", n)
if n < 100 then main' (succ n) else putStrLn "End!"
where
choose (n0, n3, n5, n15, n)
| mod n 3 == 0 && mod n 5 == 0 = n15
| mod n 5 == 0 = n5
| mod n 3 == 0 = n3
| True = n0

-
Which definition of FizzBuzz are you using? Based on the first Google result the #s are fixed at [1..100], so there shouldn't be any inputs. –  jnewman Nov 24 '12 at 19:33

You could separate your I/O from the pure code:

fizzBuzz :: Int -> String
fizzBuzz n | mod n 3 == 0 && mod n 5 == 0 = "FizzBuzz"
| mod n 5 == 0                 = "Buzz"
| mod n 3 == 0                 = "Fizz"
| otherwise                    = show n

main = mapM print (map fizzBuzz [0..100])

-
Hm. This is really good example. Thank you. –  True Nov 21 '12 at 23:26
I think mapM (print . fizzBuzz) [0..100] would be better. –  Anton Golov Nov 24 '12 at 21:08
mapM_ would be even better. –  hammar Nov 25 '12 at 13:44

jaket is definitely right, the pure/impure distinction is important. My addition would be that you should avoid recomputing the modulo:

fizzBuzz :: Int -> String
fizzBuzz n | fizz && buzz = "FizzBuzz"
| buzz         = "Buzz"
| fizz         = "Fizz"
| otherwise    = show n
where fizz = mod n 3 == 0
buzz = mod n 5 == 0

sfb = map fizzBuzz [1..15]

-
Thanks! This is good instance. –  True Nov 23 '12 at 7:00

I had written something similar to Will, but the spec I read here says, that FizzBuzz should always cover [1..100], so my implementation was a bit different:

show' :: Int -> String
show' n
| fizz && buzz = "FizzBuzz"
| buzz         = "Buzz"
| fizz         = "Fizz"
| otherwise    = show n
where fizz = mod n 3 == 0
buzz = mod n 5 == 0

fizzBuzz = [show' x | x <- [1..100]]


Will's idea about using where to cache the mod result was a nice idea IMHO.

-