<div dir="ltr">Thank you David, for taking all that time on my project. I got so much more than I was expecting. I take it my concern was focused in the wrong place? Time-space complexity of <br><pre><code><span>mapM fibb is okay?</span></code></pre><pre><code><span><br></span></code></pre></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, May 6, 2016 at 1:37 PM, David McBride <span dir="ltr"><<a href="mailto:toad3k@gmail.com" target="_blank">toad3k@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div>A lot of your functions return Either FizError a when they obviously can't have an error? I prefer to keep the functions as pure as they can be, and then lift them into Either in the few cases where I'd need them to be in that type.<br><br><div>How about fibb :: Integer -> Integer, fizzbuzz :: Integer -> Text, then in the above code:<br></div> mapM (Right <$> fizzbuzz) =<< ... =<< mapM (Right <$> fibb) =<< ...<br><br></div><div>This also greatly simplifies many of your tests and benchmarks. For example src-test/PropTests/Fibonacci.hs, testfib changes from<br><br> testfib n =<br> case (fibb n) of<br> Left _ -> False<br> Right n' -> isFib n'<br><br>to<br><br> testfib = isFib . fibb<br><br>in src-test/UnitTests/Fibonacci.hs, fibs changes from<br><br> fibs = [Right 1,Right 1,Right 2,Right 3,Right 5,Right 8,Right 13,Right 21,Right 34,Right 55]<br><br>to<br><br> fibs = [1,1,2,3,5,8,13,21,34,55]<br><br></div><div>among others.<br></div><div><br></div><div>In fizzbuzz, I would, instead of converting it to maybe and then using fromMaybe, use the option function from semigroups which is the option equivalent to maybe, and contains all the functionality of fromMaybe.<br> fizzbuzz :: Integer -> Text<br> fizzbuzz i = option (show i) id fizzbuzz'<br></div><br></div><div>Your type annotation on c = 2 and subsequent comments are unnecessary if fibb has a type annotation, which it does. I would also recommend adding types for divs and fib' as the types are non obvious and I had to use typeholes to find them.<br><br></div><div>In fibb, I would change map (toEnum . fromIntegral) to map (>0) (or /= 0). It is easier to understand, faster, and will not blow up on negative numbers (I realize there can't be, but it was not obvious to me).<br><br>You wrote an unfoldl, but I'm pretty sure you could replace that with reverse (unfoldr
...). Due to the fact that you are appending with (++) in your
implementation anyways, it is much more efficient. My final version of fibb looks like this, and passes your tests.<br> fibb = snd . foldl' fib' (1, 0) . map (>0) . reverse . unfoldr divs<br><br></div><div>boolToEither can be written with an if statement instead of case<br></div><div> boolToEither bool a b = if bool then Right b else Left a<br><br></div><div></div><div>Stylistically, I would put argument validation into src-exe and into the main function, allowing it to bail with an argument error before ever getting to your fizzbuzz code, which should not concern itself with program argument validation. I realize you are intending to demonstrate monads using either in your blog, so carry on if that's the case.<br></div><div><br></div><div>Some general advice I would give, is that if a function isn't doing as little as it can, try pulling out some of the stuff it doesn't need to do out into a shallower part of your program.<br><br>For example, when I look at the fibb where fib' function, fib' :: (a, a) -> Bool -> (a, a), where depending on the bool, and nothing else, it does two completely different things. Maybe it should be two different functions, with the bool checking outside? I'm not familiar enough with what you are trying to do to say that that can be done, but that is my first instinct.<br><br></div><div>As to the structure of your project, it looks great. I may have learned a few things...<br></div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="h5">On Fri, May 6, 2016 at 2:29 PM, Michael Litchard <span dir="ltr"><<a href="mailto:michael@schmong.org" target="_blank">michael@schmong.org</a>></span> wrote:<br></div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5"><div dir="ltr"><p>I've got this fizzbuzz project I am using for a blog series, among
other things. In this version, the fizzbuzz function is fed from a
Fibonacci generator. I'm particularly concerned with the efficiency of
the Fibonacci generator, but all scrutiny is welcomed.</p>
<p>I'll included a link to the entire project, but below are the parts I
think would be sufficient to spot trouble with how I am generating
Fibonacci numbers.</p>
<pre><code><span>-- Driver function performs following</span><span>
</span><span>-- (1) checks that input is proper</span><span>
</span><span>-- (2) creates integer list for fibonacci generator</span><span>
</span><span>-- (3) calculates first x in fibonnaci sequence</span><span>
</span><span>-- (4) generates fizzbuzz output using (3)</span><span>
fizzBuzzFib </span><span>::</span><span> </span><span>[</span><span>Text</span><span>]</span><span> </span><span>-></span><span> Either FizzError </span><span>[</span><span>Text</span><span>]</span><span>
fizzBuzzFib str </span><span>=</span><span>
mapM fizzbuzz </span><span>=<<</span><span>
mapM fibb </span><span>=<<</span><span> </span><span>-- Possible problem here</span><span>
</span><span>(\</span><span>x </span><span>-></span><span> Right </span><span>[</span><span>1</span><span> </span><span>..</span><span> x</span><span>])</span><span> </span><span>=<<</span><span>
convertToPInt </span><span>=<<</span><span>
mustHaveOne str
fibb </span><span>::</span><span> Integer </span><span>-></span><span> Either FizzError Integer
fibb n </span><span>=</span><span> Right </span><span>$</span><span> snd </span><span>.</span><span> foldl' fib' </span><span>(</span><span>1</span><span>,</span><span> </span><span>0</span><span>)</span><span> </span><span>.</span><span> map </span><span>(</span><span>toEnum </span><span>.</span><span> fromIntegral</span><span>)</span><span> </span><span>$</span><span> unfoldl divs n
</span><span>where</span><span>
unfoldl f x </span><span>=</span><span>
</span><span>case</span><span> f x </span><span>of</span><span>
Nothing </span><span>-></span><span> </span><span>[]</span><span>
Just </span><span>(</span><span>u</span><span>,</span><span> v</span><span>)</span><span> </span><span>-></span><span> unfoldl f v </span><span>++</span><span> </span><span>[</span><span>u</span><span>]</span><span>
divs </span><span>0</span><span> </span><span>=</span><span> Nothing
divs k </span><span>=</span><span> Just </span><span>(</span><span>uncurry </span><span>(</span><span>flip </span><span>(,))</span><span> </span><span>(</span><span>k </span><span>`</span><span>divMod</span><span>`</span><span> </span><span>2</span><span>))</span><span>
fib' </span><span>(</span><span>f</span><span>,</span><span> g</span><span>)</span><span> p
</span><span>|</span><span> p </span><span>=</span><span> </span><span>(</span><span>f</span><span>*(</span><span>f</span><span>+</span><span>c</span><span>*</span><span>g</span><span>),</span><span> f</span><span>^</span><span>c </span><span>+</span><span> g</span><span>^</span><span>c</span><span>)</span><span>
</span><span>|</span><span> otherwise </span><span>=</span><span> </span><span>(</span><span>f</span><span>^</span><span>c</span><span>+</span><span>g</span><span>^</span><span>c</span><span>,</span><span> g</span><span>*(</span><span>c</span><span>*</span><span>f</span><span>-</span><span>g</span><span>))</span><span>
</span><span>where</span><span>
c </span><span>::</span><span> Integer </span><span>-- See codebase for reasons</span><span>
c </span><span>=</span><span> </span><span>2<br><br></span></code>The whole project, for your critiquing eye:
<a href="https://github.com/mlitchard/swiftfizz" rel="nofollow" target="_blank">https://github.com/mlitchard/swiftfizz</a><br></pre></div>
<br></div></div>_______________________________________________<br>
Haskell-Cafe mailing list<br>
<a href="mailto:Haskell-Cafe@haskell.org" target="_blank">Haskell-Cafe@haskell.org</a><br>
<a href="http://mail.haskell.org/cgi-bin/mailman/listinfo/haskell-cafe" rel="noreferrer" target="_blank">http://mail.haskell.org/cgi-bin/mailman/listinfo/haskell-cafe</a><br>
<br></blockquote></div><br></div>
</blockquote></div><br></div>