Skip to content
This repository has been archived by the owner on Sep 20, 2023. It is now read-only.

Add connectionGet' that works like B.hGet #9

Closed
AlainODea opened this issue Dec 17, 2013 · 6 comments
Closed

Add connectionGet' that works like B.hGet #9

AlainODea opened this issue Dec 17, 2013 · 6 comments

Comments

@AlainODea
Copy link
Contributor

Here is an implementation I am using as part of in my changes to support TLS in the amqp package:

connectionGet' :: Conn.Connection -> Int -> IO BC.ByteString
connectionGet' conn x = do
  bs <- Conn.connectionGet conn x
  let diff = BS.length bs - x
  if BS.length bs == 0 || diff == 0
    then do
      return bs
    else do
      next <- connectionGet' conn diff
      return $ BC.append bs next

Relevant qualified imports:

import qualified Network.Connection as Conn
import qualified Data.ByteString as BS
import qualified Data.ByteString.Char8 as BC
@vincenthz
Copy link
Owner

I'ld rather see this as a pull request than as a comment. I'm OK in principle but got few comments:

  • the name should be GetExact (other better name can be suggested). my assumption when I see Get' is that it is some strict variant of Get.

  • no new imports needed in connection, specially Data.ByteString.Char8 is not needed (append exist in Data.ByteString)

  • I'ld rather see the recursion as an inner loop than a toplevel loop, which would factor the conn:

    connectionGetExact conn = loop
    where loop x | x == 0 = return ..
    | otherwise = do
    ...
    loop

@AlainODea
Copy link
Contributor Author

Thank you. I'll incorporate your recommendations and submit a Pull request in the next few days.

AlainODea added a commit to AlainODea-haskell/hs-connection that referenced this issue Dec 21, 2013
AlainODea added a commit to AlainODea-haskell/hs-connection that referenced this issue Dec 21, 2013
@dagit
Copy link

dagit commented Aug 11, 2016

I just ran into a problem using connectionGet and found this issue. The proposed solution works perfectly for me. Are you still planning to add connectionGetExact to the library? As a temporary solution I added connectionGetExact to my project, but it seems like a nice function to have in the official API.

@vincenthz
Copy link
Owner

sorry about the crappy delay. This shoudn't have take that long, I just didn't see it.

@AlainODea
Copy link
Contributor Author

AlainODea commented Aug 12, 2016

@vincenthz this and the family of crypto libraries around it are essential. Creating and maintaining them is a huge service to me and to the rest of the Haskell community. Thank you 👍

@dagit
Copy link

dagit commented Aug 12, 2016

I second that. Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants