https://kotlinlang.org logo
#compose
Title
# compose
s

Simon Stahl

01/13/2022, 12:29 AM
Hi everyone. I was wondering what a good solution for the
Flow operator functions should not be invoked within composition
warning is Say I have following method in my VM:
Copy code
@Composable
fun getValue() =
    flowOf(1, 2, 3)
        .map { it.toString() } // warning here
        .collectAsState(initial = "")
The naive solution is to just split the method up
Copy code
fun getValueFlow() =
    flowOf(1, 2, 3)
        .map { it.toString() }

@Composable
fun getValue() =
    getValueFlow()
        .collectAsState(initial = "")
But that is not really a functional change and if there is an issue with flow operations, then we would still have it. What would be a real fix for this warning?
i

Ian Lake

01/13/2022, 12:48 AM
Think about this way: if you recomposed every single frame (i.e., an animation is running 🙂 ), how much work are you redoing as part of composition?
Here, you're doing two things as part of composition: creating a brand new Flow (the
flowOf()
part) and applying operators on it (the
map
)
Both are those are going to be recreated and restarted on every composition, causing the
collectAsState()
from the previous composition to be cancelled (as the underlying Flow changed out from underneath it since the last composition)
So you need a way to have your
Flow
be the exact same instance on every recomposition. That means you can either make that
Flow
a
val
in your VM:
Copy code
private val valueFlow = flowOf(1, 2, 3).map {
  it.toString()
}

@Composable
fun getValue() = valueFlow.collectAsState(initial = "")
or
remember
your
Flow
so that that instance survives across recomposition:
Copy code
@Composable
fun getValue() {
  val valueFlow by remember {
    flowOf(1, 2, 3).map {
      it.toString()
    }
  }
  return valueFlow.collectAsState(initial = "")
}
That second approach is shown in the
flowWithLifecycle
example here: https://medium.com/androiddevelopers/a-safer-way-to-collect-flows-from-android-uis-23080b1f8bda#27a9
Of course, in that case, it isn't a static
flowOf(1, 2, 3)
, but a separate
val
Flow that doesn't change across recompositions - that's generally the only responsibility of the VM layer (so a
@Composable
method in a VM is a bit weird to begin with that really only makes that layer harder to test in isolation)
s

Simon Stahl

01/13/2022, 1:05 AM
Ha, interesting. I didn't notice that wrapping the flow with a
remember{}
removes the warning. That makes perfect sense of course.
And do I understand you right that a VM should not have any
@Composble
methods? So it would just provide the flow and
remember{}
as well as
collectAsState()
should be located in the composable method itself?
c

Colton Idle

01/13/2022, 1:13 AM
remember {} only goes inside of a Composable. Yeah. Ideally, your Composable are as "dumb" as possible. Think about it this way. They just take the current state that you pass into it, and they draw it. Hopefully not too much business logic or anything going on in there because you don't really control the composable re-composing. Sometimes I like to just think of it like "Hey my composable could be redrawing like 1000 times a second. I don't really know, and my composable shouldn't care" So yes, ideally all of your actually go, networking, database, other observables, go in your ViewModel. This stuff happening in your ViewModel should end up "updating" some state in your VM. And then your UI/Composable just draws the current state.
Another way is to pretend your UI doesn't exist. Pretend you're just writing an in-memory application, that only runs on the command line. Pretend that at any given point, the user of your command line app can request the output of the state of your app. If the user reads the state of the app, they should have a general sense of the current state. That's basically the VM. You do everything in there, logic, networking, etc only to end up updating the state variable you have in the VM (state = basically just a really simple model with as many properties as you need to get the point across of what state your app is in)
It's kinda nice if you think about it that way, because then you can unit test your VM. Run methods on it, and assert that your state is as expected.
The last part of your app could basically be "okay im going to take this state. pass it into a @Composable which will draw my state accurately to the user"
s

Simon Stahl

01/13/2022, 5:43 PM
right right, i couldn't agree more. and this is definitely how i have set it up. i want to make it as easy as possible for the compose view code though, that is why i would like to move the
remember
and
collectAsState
to the VM. So this is the real question right now, is that ok? because doing so requires me to annotate the VM methods with
@Composable
i can split the methods up into one that provides the flow and one that provides the state. that way the VM is still unit testable
Copy code
fun getMyValueFlow(): FLow<Int> = flowOf(1, 2, 3)

@Composable
fun getMyValueState() = 
    remember { getMyValueFlow() }.collectAsState(initial = 0)
c

Colton Idle

01/13/2022, 5:53 PM
that is why i would like to move the 
remember
 and 
collectAsState
 to the VM.
Remember is not needed in a VM. Remember is a thing thats only used in COmposables. So there is no need to wrap things in remember. Just leave off the remember {} and everything should work. The reason remember{} is needed is so that if a composable is recomposed 1000 times in 1 second, that whatever you put in the remember { } isn't called 1000 times. A VM doesn't just "recompose" and so you don't need any concept of remembering. CollectAsState should not be called in a VM. All examples from Android docs have it in the composable right before you draw your UI. I have 3 compose only apps, with probably over 80 screens across all three apps. Not one of them uses a @Composable function in a ViewModel. Now... I am definitely not a compose expert, but what you sound like you want to do seems incorrect to me (although you have good intentions where you want to alleviate the need for even a composable to have to call collectAsState)
i

Ian Lake

01/13/2022, 8:10 PM
i want to make it as easy as possible for the compose view code though, that is why i would like to move the 
remember
 and 
collectAsState
 to the VM.
I think you're just moving complexity into the wrong layer, not removing it
s

Simon Stahl

01/13/2022, 8:27 PM
got it. @Colton Idle and @Ian Lake thank you both for the input. I will rethink our current solution.
4 Views