Tusky icon indicating copy to clipboard operation
Tusky copied to clipboard

Add observe extensions for Flow to reduce nesting and boilerplates

Open Goooler opened this issue 3 months ago • 5 comments

Goooler avatar Mar 27 '24 06:03 Goooler

This depends on #4337.

Goooler avatar Mar 27 '24 06:03 Goooler

That is nice, but also quite a big change. What do you think @charlag?

connyduck avatar Mar 27 '24 11:03 connyduck

Be careful of subtle behavior changes: most of the current code doesn't use repeatOnLifecycle() but the new observe() functions do. Code that used to run once could now be repeated and in some cases that's not what you want.

cbeyls avatar Mar 30 '24 18:03 cbeyls

I think a balance needs to be found between clarity, flexibility and reducing nesting. IMO the repeating part should be optional and mentioned explicitly. I also believe it's better to mention the lifecycle explicitly for clarity.

I think having two levels of nesting is already fine:

lifecycle.launch {
    flow.collect {
    }
}

A utility function launchAndRepeatOnLifecycle could be created for cases where you want the repeating behavior:

fun Lifecycle.launchAndRepeatOnLifecycle(
    state: Lifecycle.State = Lifecycle.State.STARTED,
    block: suspend CoroutineScope.() -> Unit
): Job = coroutineScope.launch {
    repeatOnLifecycle(state, block)
}

fun LifecycleOwner.launchAndRepeatOnLifecycle(
    state: Lifecycle.State = Lifecycle.State.STARTED,
    block: suspend CoroutineScope.() -> Unit
): Job = lifecycle.launchAndRepeatOnLifecycle(state, block)

Then to collect repeatedly, also two levels of nesting:

lifecycle.launchAndRepeatOnLifecycle {
    flow.collect {
    }
}

Note that you can also collect multiple things inside a lifecycle repetition by launching child coroutines. This is also more efficient than launching multiple instances of repeatOnLifecycle():

lifecycle.launchAndRepeatOnLifecycle {
    launch {
        flow1.collect {
        }
    }
    launch {
        flow2.collect {
        }
    }
}

In that case it would indeed create 3 levels of nesting. But the collecting code could also be moved to separate suspending functions to improve readability.

cbeyls avatar Mar 30 '24 18:03 cbeyls

Be careful of subtle behavior changes: most of the current code doesn't use repeatOnLifecycle() but the new observe() functions do.

That is what I'm considering too, remove it for now.

Goooler avatar Mar 31 '24 02:03 Goooler