<div dir="ltr">On Tue, Apr 19, 2016 at 11:14 AM, Dániel Arató <span dir="ltr"><<a href="mailto:exitconsole@gmail.com" target="_blank">exitconsole@gmail.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi guys,<br>
<br>
Here is my solution to Problem 50 from Ninety-Nine Haskell Problems:<br>
<a href="https://github.com/nilthehuman/H-99/blob/master/Logic.hs#L75" rel="noreferrer" target="_blank">https://github.com/nilthehuman/H-99/blob/master/Logic.hs#L75</a><br>
<br>
(`consume' is defined here:<br>
<a href="https://github.com/nilthehuman/H-99/blob/master/Lists.hs#L107" rel="noreferrer" target="_blank">https://github.com/nilthehuman/H-99/blob/master/Lists.hs#L107</a> )<br>
<br>
I'm not satisfied with the code quality. I feel like especially `go',<br>
`group' and `min2' should be more succint and readable.<br></blockquote><div><br>Dániel,<br>Looking at your code, there are several things that I note immediately.<br>First, your 'consume' combinator can be rewritten as<br>> consume f g a = foldl f a . chunk g<br>> chunk g = unfoldr (fmap g . (\xs -> if null xs then Nothing else Just xs))<br><br>Second, you may want to reconsider your data model. As it is, your code is<br>inefficient, since you are simulating priority queues using lists.<br>Not everything can be solved naturally with lists.<br><br>Assume we proceed anyway with your model, despite these misgivings. Note that<br>any operation on your symbols is most naturally expressed in terms of sets of<br>symbols with a common root. Hence, you may want to use<br>> gatherRoots = chunk (partition (compare `on` root))<br>to split your queue into these sets. Note that this also gives you an easier way<br>of expressing 'done' as<br>> done = (<=1) . length . gatherRoots<br><br>Once that is obtained, you want the minimal two sets by weight. Hence:<br>> (x:y:xs) = map fst . sortBy (compare `on` snd) . map (id &&& flatten)<br>>   where flatten = sum . map weight<br>>         weight (_,w,_,_) = w<br><br>Finally, you want to<br>> go xs = map (update '0') x ++ map (update '1') y ++ xs<br>>   where update p (c,w,ps,_) = (c,w,p:ps, rn)<br>>         rn = rNext xs<br><br>Note that this also avoids the ugly nested-if you had there.<br><br>I played around with your code and refactored it to my taste. I extracted the<br>main priority-queue simulation code into a typeclass instance so that you can<br>see that the majority of the complexity of your code comes from simulating<br>priority queues using lists. The completed code is here [0]. Note that I've<br>taken some liberties with the naming and refactoring, and this may not all be to<br>your taste. YMMV.<br><br>I hope this helps, and that the criticism I gave was correct and will be<br>well-received.<br><br>Regards,<br>Gesh</div><div><br>P.S. You would be correct in claiming that this rewrite is too distant from the<br></div><div>original to be of use. My apologies if this is the case.<br></div><div><br>[0] - <a href="https://gist.github.com/anonymous/cd4e21105676894dcd579fcf8d4c4b41">https://gist.github.com/anonymous/cd4e21105676894dcd579fcf8d4c4b41</a><br> </div></div></div></div>