Fix replace()/replace!() for ChainedVector when supplying count#113
Fix replace()/replace!() for ChainedVector when supplying count#113laborg wants to merge 0 commit intoJuliaData:mainfrom
Conversation
ararslan
left a comment
There was a problem hiding this comment.
Implementation looks good I think, but it'd be nice to have tests for replace (currently they're only for replace!), for the error thrown when count < 0, and for the behavior when count == 0.
src/chainedvector.jl
Outdated
| if R[ri] == old && reps < count | ||
| R[ri] = new | ||
| reps += 1 | ||
| continue |
There was a problem hiding this comment.
| continue |
The continue has no effect here; there's nothing else in the loop body that appears after it so the loop will continue regardless.
src/chainedvector.jl
Outdated
| if A[i] == old | ||
| A[i] = new | ||
| reps += 1 | ||
| continue |
There was a problem hiding this comment.
| continue |
(Same comment as in the other function)
|
Is there any reason you opted not to implement |
|
Replaced by #118 |
Fix #112
Not sure if this is the fastest implementation, but now it does what I would have expected from it and is in line with the behaviour of replace/replace! on an ordinary
Array