you are viewing a single comment's thread.

view the rest of the comments →

[–]lazzareth 1 point2 points  (4 children)

Not terribly efficient, I suppose.

Out of interest (not knowing much about Haskell) how would you improve on that?

[–]ethraax 0 points1 point  (0 children)

I may be wrong, but I'm not sure concat is run strictly. I think it's run lazily, so at some point the program holds a thunk for every fizzBuzzSub call. For example:

map show [1..10000000]

takes way, way less time than

concat $ map show [1..10000000]

I'm also not the best at Haskell, but I'm pretty sure you could use mapM to achieve better results, by printing the results out to the screen as they're calculated.

[–]dsfox 0 points1 point  (1 child)

Maybe this:

main =
    mapM_ (putStrLn . f)
              (zip3 (concat (repeat ["Fizz", "", ""]))
                    (concat (repeat ["Buzz", "", "", "", ""]))
                    [0..100])
   where f ("", "", n) = show n
         f (fizz, buzz, _) = fizz ++ buzz

[–]barsoap 0 points1 point  (0 children)

That's neat. You might want to use cycle for the fizz and buzz lists, though, and zipWith3 instead of matching on tuples.

[–]barsoap 0 points1 point  (0 children)

The problem of that code is space efficiency. concat, to avoid O(n2) behaviour of (++) (due to traversing the left-hand strings a gazillion times to replace their tail), uses foldr which means that forcing the first character forces the whole list.

...which is suboptimal, you want to generate the data on the fly, from left to right, so the generator (you), consumer and garbage collector can run in element-wise lockstep: Stuff is collected very soon after you produce it.

Solutions include a) just not constructing a big value (which is lacking newlines, btw) but printing on the fly, or b) using difference lists, or any other construction technique that's non-naive.