New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add function Data.Conduit.List.chunksOf (issue #295) #296

Merged
merged 3 commits into from Feb 8, 2017

Conversation

Projects
None yet
2 participants
@tomsmalley
Contributor

tomsmalley commented Feb 7, 2017

Introduced an additional dependency in the test suite.
Stream version not included.
See issue #295

--
-- Subject to fusion
--
-- Since ???

This comment has been minimized.

@snoyberg

snoyberg Feb 7, 2017

Owner

Can you put this at 1.2.9, and adjust the .cabal file and ChangeLog accordingly?

where
start = await >>= maybe (return ()) (loop n id)
loop count rest x =

This comment has been minimized.

@snoyberg

snoyberg Feb 7, 2017

Owner

May be worthwhile to use a bang pattern on count (i.e., !count) to avoid laziness. The guard below should be enough for the strictness analyzer, but better safe than sorry.

chunksOf n =
start
where
start = await >>= maybe (return ()) (loop n id)

This comment has been minimized.

@snoyberg

snoyberg Feb 7, 2017

Owner

Instead of passing the x around below, how about:

start = await >>= maybe (return ()) (\x -> loop n (x:))

You could point-free that... but please don't, it'll hurt my head :)

await >>= maybe (yield (x : rest [])) go
where
go y
| count > 1 = loop (count - 1) (rest . (y:)) x

This comment has been minimized.

@snoyberg

snoyberg Feb 7, 2017

Owner

Very nice way to avoid the negative-input case 👍

@@ -235,6 +236,20 @@ main = hspec $ do
C.=$ CL.fold (+) 0
x `shouldBe` 2 * sum [1..10 :: Int]
it "chunksOf" $ do
let input = [1..16] :: [Int]
x <- runResourceT $ CL.sourceList input

This comment has been minimized.

@snoyberg

snoyberg Feb 7, 2017

Owner

I think the runResourceT is unnecessary here.

@@ -235,6 +236,20 @@ main = hspec $ do
C.=$ CL.fold (+) 0
x `shouldBe` 2 * sum [1..10 :: Int]
it "chunksOf" $ do
let input = [1..16] :: [Int]

This comment has been minimized.

@snoyberg

snoyberg Feb 7, 2017

Owner

How about turning this into a prop with a variable size? Or add that as an additional case?

it "chunksOf (negative)" $ do
let input = [1..16] :: [Int]
x <- runResourceT $ CL.sourceList input

This comment has been minimized.

@snoyberg

snoyberg Feb 7, 2017

Owner

Also not needed here

@snoyberg

This comment has been minimized.

Owner

snoyberg commented Feb 7, 2017

Looks good!

@tomsmalley

This comment has been minimized.

Contributor

tomsmalley commented Feb 7, 2017

Changes implemented!
Let me know if I'm missing something.
edit: Missed most of the files, see following commit

@snoyberg snoyberg merged commit e2894d3 into snoyberg:master Feb 8, 2017

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@snoyberg

This comment has been minimized.

Owner

snoyberg commented Feb 8, 2017

Looks great, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment