k0001 / pipes-binary

Encode and decode binary streams using the pipes and binary libraries.
Other
7 stars 12 forks source link

Add `encoder`? #13

Closed Gabriella439 closed 10 years ago

Gabriella439 commented 10 years ago
encoder :: (Monad m, Binary a) => Pipe a ByteString m r
encoder = for cat encode

This ties into https://github.com/k0001/pipes-binary/issues/12, where providing this will guide the user to the idiomatic solution for encoding.

If you're fine with this, then I can write it up into a pull request.

Gabriella439 commented 10 years ago

I just realized that this doesn't type-check unless you generalize encode to be a Producer'. So that should be included as part of this proposal.

k0001 commented 10 years ago

Producer' is fine, let's generalize that.

However, I'm not convinced we should export this function “as a convenience”, as it adds yet another function with the name “encode” in it to the library, which is a bit confusing. And if we add this, we probably should add encoderPut too so that you can use this function for types that lack a Binary instances.

My suggestion, instead, would be to add some documentation saying “use for cat encode to achieve this and that”. This is the same approach I used here: http://hackage.haskell.org/package/pipes-aeson-0.2.1/docs/Pipes-Aeson.html#v:encode

What do you think?

Gabriella439 commented 10 years ago

Yeah, that works fine for me. Can you also add a short-cut fusion rule like this:

{-# RULES "p >-> for cat encode" forall p .
    p >-> for cat encode = for p encode
  #-}
k0001 commented 10 years ago

I will. Thank you for that hint!

k0001 commented 10 years ago

I've changed the docs and added the RULES in 194b91e, now in master. Thank you.

Gabriella439 commented 10 years ago

Thanks to you, too!