Add dropEnd and takeEnd #130
Conversation
safareli
left a comment
safareli
left a comment
There was a problem hiding this comment.
👍 just a small issue in test comments
test/Test/Data/List.purs
Outdated
| assert $ (take 2 (l [1, 2, 3])) == l [1, 2] | ||
| assert $ (take 1 nil) == nil | ||
|
|
||
| log "take should keep the specified number of items from the end of an list, discarding the rest" |
There was a problem hiding this comment.
takeEnd should keep the specified number of items from the end of an list, discarding the rest
test/Test/Data/List.purs
Outdated
| assert $ (drop 2 (l [1, 2, 3])) == l [3] | ||
| assert $ (drop 1 nil) == nil | ||
|
|
||
| log "drop should remove the specified number of items from the front of an list" |
There was a problem hiding this comment.
dropEnd should remove the specified number of items from the end of an list
| -- | Running time: `O(2n - m)` where `n` is the number of elements in list | ||
| -- | and `m` is number of elements to take. | ||
| takeEnd :: forall a. Int -> List a -> List a | ||
| takeEnd n xs = drop (length xs - n) xs |
There was a problem hiding this comment.
This involves two traversals of the list, which we should try to avoid. Let's try to write these using foldr instead please.
There was a problem hiding this comment.
Is it even possible for this function to have better complexity?
in order to take n elements from the end we need to start iterating the list from the start till we reach length - n position and return the tail. So we require length (whole list iteration) and we need to determine the index we stop on (length - n elements iteration)
There was a problem hiding this comment.
Yeah, ignore me, this is fine as it is. I thought we could do it in one pass with a buffer of size n, but it's probably not worth it. We can revisit later if necessary.
There was a problem hiding this comment.
Alternative implementation will be to first reverse list and then take. it's performance will be n + m. it will be better then current version when m is smaller then n/2.
There was a problem hiding this comment.
first reverse list and then take and then reverse
There was a problem hiding this comment.
oh yeh :D so that will be better when m is less then n/4. so never mind
test/Test/Data/List.purs
Outdated
| assert $ (takeWhile (_ /= 1) nil) == nil | ||
|
|
||
| log "drop should remove the specified number of items from the front of an list" | ||
| log "dropEnd should remove the specified number of items from the end of an list" |
There was a problem hiding this comment.
You made this change in for wrong string :p
|
Looks good now? |
| assert $ (takeWhile (_ /= 1) nil) == nil | ||
|
|
||
| log "drop should remove the specified number of items from the front of an list" | ||
| log "dropE should remove the specified number of items from the front of an list" |
There was a problem hiding this comment.
drop not dropE :)
But this is fine.
|
Great, thanks! |
3 participants
Similar to the purescript/purescript-arrays#113