On Fri, Jul 04, 2014 at 09:36:20AM -0700, Gabriel Gonzalez wrote:
> Comments inline below:
> 
> >So it just splits the Producer into groups, and folds them back together with
> >mappend. This works exactly as expected, lovely, however it's quite verbose 
> >and
> >possibly over-complicated?
> 
> I think this is as good as it is going to get.  My rule of thumb for these
> things is "would the equivalent code on ordinary lists be simpler?"  In
> other words, would it be easier to implement the following type:
> 
>     builderChunks :: Int -> [(Int, Builder)] -> [[Builder]]
> 
> The solution for lists would probably look very similar, recursing over the
> list by hand, accumulating the length of bytes seen so far.  It might be
> smaller by a constant factor (since pattern matching on lists is
> syntactically cheaper than "pattern matching" on producers using `next`),
> but the overall algorithm would be roughly the same.

That makes sense.

> 
> >The actual implementation can be found here:
> >https://github.com/anchor/marquise/blob/master/lib/Marquise/Server.hs
> 
> Side note: your code is very readable!

Thank you!

> >By the way, can anyone tell me how to get the type annotation on "go" to
> >typecheck?
> >
> >My question: It seems like builderChunks could be re-usable. Possibly
> >              implemented as a lens similar to chunksOf?
> >
> >   --| someAwesomeName is a lens that splits a Producer into a FreeT group of
> >   --  Producers of at least the minimum size provided.
> >   someAwesomeName :: Monad m
> >                   => Int
> >                   -- ^ Minimum size
> >                   -> Lens' (Producer (Int, a) m x) (FreeT (Producer a m) m 
> > x)
> 
> I think that is a little too specialized.  However, there may be a more
> useful generalization of that idea that would be sort of like a `takewhile`
> lens.

Right, I would have found that very useful. I would need an accumulator to keep
state, so something like?

        takesWhile' :: Monad m
                    => (acc -> a -> Bool)
                    -> acc
                    -> Lens' (Producer a m x) (FreeT (Producer a m) m x)

Where the accumulator is reset and a new group is started at each False.
This is somewhat similar to groupsBy:

        groupsBy :: Monad m
                 => (a -> a -> Bool)
                 -> Lens' (Producer a m x) (FreeT (Producer a m) m x)

I wouldn't really call this takesWhile, takesWhile would be:

        takesWhile :: Monad m
                   => (a -> Bool)
                   -> Lens' (Producer a m x) (FreeT (Producer a m) m x)

Which could maybe be defined in terms of takesWhile', which should get
optimised right out?

takesWhile f = takesWhile' (const f) undefined

Anyway, I'd like to add these lenses if you deem this satisfactory?  We would
need a better name for takesWhile', maybe groupWithAcc?

> >Another question: Would it be reasonable to expose the splitting part of the
> >lenses in your pipes-parse into separate functions?
> 
> You can depend on `lens-family-core`, which is a really tiny dependency,
> instead of `lens`.

I'll just use that dependency then, thanks for the input!

-- 
Christian Marie - Sparkly Code Princess

Attachment: pgpNabLB4h6S5.pgp
Description: PGP signature

Reply via email to