A better FizzBuzz

Question

Is there any way I can make this FizzBuzz better?

toFizz :: Int -> String
toFizz x =
  case (mod x 3, mod x 5) of
      (0, 0) -> "FizzBuzz"
      (0, _) -> "Fizz"
      (_, 0) -> "Buzz"
      (_) -> show x

main :: IO()
main =
    mapM_ (print . toFizz) [1..100]

Answer 1

Sure, I can suggest two improvements:

One is rewriting toFizz in a more explicit manner:

toFizz :: Int -> String
toFizz x
  | x `mod` 15 == 0 = "FizzBuzz"
  | x `mod` 3 == 0 = "Fizz"
  | x `mod` 5 == 0 = "Buzz"
  | otherwise = show x

Your pattern match with (mod 3, mod 5) puzzled me a fair bit. Also use

x `mod` y

Instead of:

mod x y

for readability.

main can be written to avoid all those quotes making output nicer:

main :: IO()
main = putStrLn $ unlines $ map toFizz [1..100]

Answer 2

As gallais has mentioned, the cost of your calculations increases as x increases.

I'm always struck by how few people, when faced with fizzBuzz or similar challenges (like finding the first n numbers which are divisible by 3 and/or 5), realise there is no need to work backwards from x to the original primes. It's much more efficient to work forwards. After all, you know the starting primes, 3 and 5. One might consider, for example, generating a map where the keys are the numbers 1 to 100 (or x to y) and the value is show. Then iterate over the multiples of 3 between x and y, replacing the value with const "fizz" and so on. Then iterate over the keys of the map in order, applying the value (a function) to the key. There are smarter (and more efficient) ways to work forwards, but it illustrates the principle.

Here's a relatively naive forwards-iterating implementation of toFizzBuzz I just thought up.

fizz :: Int -> String
fizz = const "fizz"

buzz :: Int -> String
buzz = const "buzz"

fizzbuzz :: Int -> String
fizzbuzz = const "fizzbuzz"

fizzbuzzFuncs =  cycle [show, show, fizz, show, buzz, fizz, show, show, fizz, buzz, show, fizz, show, show, fizzbuzz]

toFizzBuzz :: Int -> Int -> [String]
toFizzBuzz start count =
    let offsetFuncs = drop (mod (start - 1) 15) fizzbuzzFuncs
    in take count $ zipWith ($) offsetFuncs [start..]

Note that it can work from any starting point (even negative n) for any range, within the limits imposed by Int. Now, there are smarter ways to do this kind of thing (and certainly more idiomatic and generalised), but that example is still more efficient than 99% of the fizzBuzz attempts I see, Only does any arithmetic once.

Work forwards, not backwards. Don't go looking expensively for things you can easily generate.

Challenge: consider a solution using unfoldr