Don't force more than necessary in Data.List.Lazy take #139
Conversation
src/Data/List/Lazy.purs
Outdated
| @@ -509,10 +509,10 @@ slice start end xs = take (end - start) (drop start xs) | |||
| -- | | |||
| -- | Running time: `O(n)` where `n` is the number of elements to take. | |||
| take :: forall a. Int -> List a -> List a | |||
| take n = List <<< map (go n) <<< unwrap | |||
| take n _ | n <= 0 = nil | |||
| take n xs = List <<< map (go n) <<< unwrap $ xs | |||
There was a problem hiding this comment.
Tiny nitpick: You could probably just do $ unwrap xs, no?
There was a problem hiding this comment.
(Or maintain the point-free form used previously.)
|
This looks good to me. I added a tiny comment about style. |
|
👍 for keeping the previous pointfree form. Also would you mind adding a test? Perhaps something like this? > oops x = oops x
> xs = defer \_ -> 1 : defer oops
> take 1 xs == fromFoldable [1]The idea being this will test that we don't evaluate any more of the list than needed, since if we tried to, it would crash with a stack overflow. |
|
Thanks for the replys.:smiley: take :: forall a. Int -> List a -> List a
take n | n <= 0 = const nil
take n = List <<< map (go n) <<< unwrapBut I think it is a little difficult to understand, since it used an additional |
|
Yes, although come to think of it I think using |
|
Done.:smiley:
(I use |
test/Test/Data/List/Lazy.purs
Outdated
| @@ -270,6 +270,11 @@ testListLazy = do | |||
| assert $ (take 2 (l [1, 2, 3])) == l [1, 2] | |||
| assert $ (take 1 nil') == nil' | |||
|
|
|||
| log "take should evaluate exactly n items which we needed" | |||
| assert let oops x = 0 : (oops x) | |||
There was a problem hiding this comment.
sorry to nitpick here, but the parens around oops x aren't necessary and I think it would be better style to remove them, and also I think it would be a little nicer (and more in line with the style used elsewhere in this file) to use parens instead of $ here too.
There was a problem hiding this comment.
So I see! parens are not unnecessary here. Thanks for your correction.
* Remove parens * Remove `($)` by let xs = 1 : defer oops
|
Fixed.
|
hdgarrood
changed the title
#138
Made a fix of
take.