Add ReceiveChannel.toList(destination)

[dkhalanskyjb]

Use case: collecting elements up until the point the channel is closed without losing the elements when toList when the exception is thrown.

This function is similar to Flow<T>.toList(destination), which we already have, so the addition makes sense from the point of view of consistency as well.

[murfel]

For context, Flow's toList(destination) that you've referenced

https://kotlinlang.org/api/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines.flow/to-list.html

https://github.com/Kotlin/kotlinx.coroutines/blob/master/kotlinx-coroutines-core/common/src/flow/terminal/Collection.kt#L11

[murfel]

Re the original toList() docmuentation - shall it say that toList must be used by a single consumer (the implication of consumeEach and the iterator used there)?

[dkhalanskyjb]

It already says so:

• The operation is terminal.
• This function [consumes][ReceiveChannel.consume] all elements of the original [ReceiveChannel].

[murfel]

Yes, but the UX is bad and in its current form people will use it incorrectly with a higher probability.

Terminal doesn't redirect anywhere.

Oh wait, you can technically call it from different coroutines, it just doesn't make much sense. Nevermind then.

[murfel]

Oh, we can't check that the return value will be used now. Maybe unsplit it back, and do not return List on one of the destination overload?

[dkhalanskyjb]

I've attempted this, but cross-referencing overloads clutters the documentation quite a bit, and also, Flow.toList already follows the single-overload approach: public suspend fun <T> Flow<T>.toList(destination: MutableList<T> = ArrayList()): List<T>, so it's more internally consistent to have a single overload.

I'm wondering how real the risk of calling toList() on a channel and forgetting to obtain the resulting list is. Do you have an opinion on this?

[murfel]

We could in theory check it on flow if people do this mistake...

However, having the return value also allows chaining, and that tips the scale for me.

So let's keep your current version.

[murfel]

I wonder if people mistakenly do flow.toList() instead of flow.collect() to trigger flow execution for its side effects.

[dkhalanskyjb]

@sandwwraith, what do you think, is it more idiomatic to have fun X.toList(): List<T> + fun X.toList(destination: MutableList<T>): Unit or just fun X.toList(destination: MutableList<T> = ArrayList<T>): List<T> from the point of view of having an excessive return value?

[sandwwraith]

I think it is better to have it as in the stdlib now: fun X.toList(): List<T> + @Ignorable fun <C: Collection/MutableList> X.toCollection/MutableList(destination: C): C. Generic argument in the second function allows you to collect result to ArrayList or HashSet, for example.

[murfel]

Newline police