tickerFlow implementation
#1908
Conversation
testZeroInitialDelay added to test 0 initialDelay. testDoNotReceiveAfterCancel testcase fixed.
ibrahimyilmaz
changed the title
ibrahimyilmaz
changed the title
ibrahimyilmaz
changed the title
tickerFlow implementation
| */ | ||
| public fun tickerFlow( | ||
| period: Long, | ||
| initialDelay: Long = period |
There was a problem hiding this comment.
To follow the example of functions like withTimeout and debounce, I think we should add the Millis suffix to these parameter names, and maybe provide an overload using kotlin.time.Duration?
There was a problem hiding this comment.
Good point for naming and we have .receiveAsFlow() extension function.
Directly we can use kotlinx-coroutine's ticker. Also this can be better idea to have single implementation.
What do you think about it?
There was a problem hiding this comment.
It could indeed be done this way, but that would couple it to JVM platform 😥
I am not sure a channel would be necessary behind the scenes to implement it, but I will let the maintainers comment on this.
EDIT: moved part of this comment to the relevant conversation
| val timer = Timer() | ||
| timer.schedule(initialDelay, period) { | ||
| offer(Unit) | ||
| } |
There was a problem hiding this comment.
This implementation uses a background thread and is JVM-specific.
Couldn't we simply use a loop with delay() and make it multiplatform? Or am I naive here?
I fail to see the benefit of running another thread here.
There was a problem hiding this comment.
Thanks! very good point.
Also ticker implementation can be moved to commonMain.
There was a problem hiding this comment.
I can see now why a concurrent behaviour is necessary and would like to backtrack a bit: we can't use a simple loop with delays because otherwise the ticker delay is impacted by the execution time of the collector code, which is probably not desirable in this case.
That's why we do need some concurrent process (a coroutine?) to send the ticks.
This still doesn't mean we have to use a thread and Java's timer.
There may be a multiplatform way to do this.
I'll let the maintainers of the lib comment on this, though
… receiveAsFlow extension function.
| mode: TickerMode = TickerMode.FIXED_PERIOD | ||
| ): Flow<Unit> { | ||
| require(delayMillis > 0) | ||
| return ticker(delayMillis, initialDelayMillis, context, mode).receiveAsFlow() |
There was a problem hiding this comment.
Because the channel is hidden here, we probably want to handle cancellation properly.
Using consumeAsFlow() instead of receiveAsFlow() should do the trick I think.
This pull request contains basic implementation of the ticker with Flow.
I have seen this issue(#540) maybe this can contribute to that issue(plan)