Add intersperse with tests #188
Conversation
|
Can I get a review here? |
src/Data/Array.purs
Outdated
| -- | ``` | ||
| -- | | ||
| -- | If the array has one or zero elements, the resulting array is a copy | ||
| -- | of the input array. |
There was a problem hiding this comment.
I decided to use copy since it's not technically the "same" array.
There was a problem hiding this comment.
It actually will be the same array with this implementation. If you mutate one, you’ll see that change reflected in the other.
There was a problem hiding this comment.
Ah... Shoot, that's right. Let me fix that.
There was a problem hiding this comment.
Fixed in latest commits
There was a problem hiding this comment.
I think the previous implementation was preferable - there's no reason to make a copy here, since Array is already immutable. You'd have to use an escape hatch like unsafeThaw or the FFI to mutate an Array, but that's not something we need to defend against, because it can only happen through misuse of said escape hatches.
There was a problem hiding this comment.
True.
Latest commit reverts that change
This doesn't work currently, but it's forward-compatible when we do implement syntax highlighting
|
CI builds. |
src/Data/Array.purs
Outdated
| -- | intersperse 0 [ 1, 2, 3, 4, 5 ] == [ 1, 0, 2, 0, 3, 0, 4, 0, 5 ] | ||
| -- | ``` | ||
| -- | | ||
| -- | If the array has one or zero elements, the input array is returned. |
There was a problem hiding this comment.
I think "less than two" might read a bit more naturally here? Up to you.
There was a problem hiding this comment.
I don't know. I think I could go either way.
Meh, let's just use the "less than two."
src/Data/Array.purs
Outdated
| let unsafeGetElem idx = unsafePartial (unsafeIndex arr idx) | ||
| out <- STA.empty | ||
| _ <- STA.push (unsafeGetElem 0) out | ||
| STI.for 1 len \idx -> do |
There was a problem hiding this comment.
I think for is re-exported from Control.Monad.ST, so I don't think we need to import the Internal module here (and if we can avoid importing the Internal module, it would be good to do that).
There was a problem hiding this comment.
Ah! Good catch! Fixed in latest commits.
Fixes #15