Is there any difference between this two implement...

# coroutines

[zokipirlo]

Is there any difference between this two implementations? Which solution is preferred?
1.

supervisorScope{
    list.map {
        async { doSomeApiCall(it) }
    }.awaitAll()
}

2.

supervisorScope {
    list.map {
        async { doSomeApiCall(it) }
    }
}.awaitAll()

[ephemient]

the .awaitAll() isn't needed either way because a scope does not end until all its children are completed

[Joffrey]

@ephemient awaitAll still converts Deferred into their inner value, and I guess the code is incomplete here but I'm assuming the OP would want to get those values. However I agree that it doesn't await anything anymore, they will all be immediately converted. If the result of awaitAll() is really ignored like in the exemple, it's indeed pointless.

[Joffrey]

It can behave differently if there is other code inside the supervisorScope after map . In the first case, the extra code would be executed after the map, while in the second case it will run concurrently with the map. But there is no such code in your example

[Joffrey]

I would personally prefer the first option, because it's clearer that you await on list returned by the map. It's less visible when applied to supervisorScope. Also I like to see supervisorScope as blocks, so calling a function on it seems weird.

[Sam]

👍 stylistically, I prefer option 1, because as @ephemient said, the jobs will all be complete by the time supervisorScope returns anyway. If you were to await them outside the scope, you're really just unwrapping them after they're already finished. Awaiting them inside the scope is more representative of what will actually happen.

[zokipirlo]

I thought awaitAll is needed because it starts async block and without awaitAll nothing will execute.
All examples I saw was with async block, that's why I did it like that. I guess I could change async to launch because result from suspend function is ignored in my case?

[Joffrey]

If you don't need results, then you can indeed simplify by removing awaitAll(). Also, launch is better because it expresses that you don't care about the result

[Sam]

The default behaviour of async is to start eagerly, just the same as launch (you can change that by passing a CouroutineStart argument). But there are some circumstances where that still might not start immediately, for example when using runBlocking, so maybe that accounts for the examples that you saw.

[bezrukov]

One more difference that wasn't mentioned yet - immediate awaitAll will fail as soon as any of async calls fails. The second variant will wait for the completion of all async calls, even if some of them fail (because of supervisor scope)

[ephemient]

just a bit more nuance: the first variant will fail when the first (in list order) async call fails, and the bubbled exception will cancel any remaining async calls still in flight. because it is contained in supervisorScope, other async calls may fail without notice. which is somewhat similar but not the same as the behavior of

coroutineScope {
    list.forEach {
        launch { doSomeApiCall(it) }
    }
}

which will cancel any remaining calls when the first (in execution order) launch fails.

[bezrukov]

In list order

It is not true, awaitAll fails once the first in execution order fails, from the doc:

This function is not equivalent to deferreds.map { it.await() } which fails only when it sequentially gets to wait for the failing deferred, while this awaitAll fails immediately as soon as any of the deferreds fail.

[ephemient]

TIL. thanks

[zokipirlo]

So the safest solution is with launch because I don't want others be cancelled if any exception happens?

[ephemient]

if that's what you want, then yes, launch and don't explicitly .join() the jobs

[gildor]

So the safest solution is with launch because I don’t want others be cancelled if any exception happens?

If this is your explicit requirement, I wouldn’t bother with supervisorScope at all and instead wrap doSomeApiCall to try/catch and required error handling