https://kotlinlang.org logo
Join Slack
Powered by
Hello, everyone! :blush: I have a `Flow` inside a...
# compose
e
Hello, everyone! 😊 I have a 
Flow
inside a 
ViewModel
that is notified in every time a change is made in my database and reflect the state via 
sealed class
to my composable. This 
Flow
is attached to the 
viewModelScope
. Basically I’m facing two problems: 1. Every time the screen is recomposed, the function to load the data is called and a new flow is registered and listening to changes. 2. When I’m navigating in another composables that handles the same data, the 
Flow
keeps emitting data changes for the composable that is no longer visible. Is there a better way to implement it? Should I use my own scope instead the one from 
ViewModel
? If so, how can I “dispose” the scope when the Composable is no longer visible?
Copy code
@Composable
private fun TaskListLoader(viewModel: TaskListViewModel = viewModel()) {
    viewModel.loadTasks()
    val viewState by viewModel.state.collectAsState()
    TaskScaffold(...)
}
Copy code
fun loadTasks() = viewModelScope.launch {
    loadAllTasksUseCase().collect { tasks ->
        // emit via StateFlow
    }
}
Thanks a lot in advance! ❤️
a
you are calling 
viewModel.loadTasks()
every recompose so it repeats infinitely
so just run on it on initial composition? I run my loads in 
ViewModel.init {}
You need to scope your ViewModel if you want separate instances. you can use navigation-compose to do that.
e
Yes, that’s seems related to the first issue… But how can I do it? Split the 
viewModel.loadTasks()
and 
TaskScaffold
in different functions?
a
I edited above ^
e
I can move this specific one to 
init{}
but there are some functions that needs params.
b
especially the 
LaunchedEffect
e
It seems like a good start to fix both problems. 😊 I’m just have some difficult to understand all the different types of Effects. But thanks a lot!
Any ideas if this is a good approach?
Copy code
@Composable
private fun TaskListLoader(viewModel: TaskListViewModel = viewModel()) {
    LaunchedEffect(viewModel) {
        viewModel.loadTasks()
    }
    val viewState by viewModel.state.collectAsState()
    TaskScaffold(...)
}
I made the 
ViewModel
function suspend and only loading it again if the ViewModel changes.
Also, it is no longer is notified if the data changes in other Composable. 😄
a
It can be considered a code smell. You can check around the slack channel as to why
a
Why wouldn't 
viewModel.state
emit when needed and handle the cold subscription properties here?
Any variant of 
viewModel.loadTasks()
seems like it should be redundant with 
viewModel.state.collectAsState()
e
Do you mean like calling my 
loadTasks
inside 
init{}
and only expose the 
State
?
a
ideally the 
state
Flow
would handle it as a result of handling the collect operation. ViewModels that launch in their init blocks are creepy for a whole host of other reasons before Compose even enters the picture
if you need it to be a StateFlow, instead of it being a 
MutableStateFlow
that other things write into arbitrarily, use a standard 
flow
or 
channelFlow
combined with 
.stateIn(viewModelScope, SharingStarted.WhileSubscribed, initialValue)
then the 
flow
or 
channelFlow
will start lazily when at least one subscriber is present and stop when all subscribers go away. Launching in a ViewModel's init block will stay running for a very long time, whether or not the app is even in the foreground
Several years back there was a big push in the RxJava/Android community to stop leaning on `Subject`s and instead make more explicit use of cold observables. We're starting to see the same thing with 
Mutable[State/Shared]Flow
now, and it's going to result in the same kind of pushback 🙂
b
@Adam Powell I'm really interested about this trend (pros / cons) if you have more info. Don't know if this channel is the best place though ^^
a
Eh, this deep in a thread I don't mind so long as other thread participants don't 🙂
👍 3
e
Thanks for your patience, Adam. I’m sorry if I asking things that are obvious. I’ll try to investigate your points and implement something here.
I tried and seems to work as expected now:
Copy code
val state: Flow<TaskListViewState> = flow {
    loadAllTasksUseCase()
        .map { task -> taskWithCategoryMapper.toView(task) }
        .catch { error -> emit(TaskListViewState.Error(error)) }
        .collect { tasks ->
            val result = if (tasks.isNotEmpty()) {
                TaskListViewState.Loaded(tasks)
            } else {
                TaskListViewState.Empty
            }
            emit(result)
        }
}
Copy code
@Composable
private fun TaskListLoader(viewModel: TaskListViewModel = viewModel()) {
    val viewState by viewModel.state.collectAsState(initial = TaskListViewState.Empty)

    TaskListScaffold(...)
}
In my head I want a very simple thing: I have a 
Flow<Task>
and a 
Composable
. I want the 
Composable
to observe the 
Flow
, without duplication or observing it forever… It shouldn’t be so complex. However we receive so much missed signals, that this simple implementation becomes a nightmare.
Some examples uses 
State
, other uses 
StateFlow
, now you introduced me to 
Flow
for this scenario (if my implementation is correct). I’m sure if I search hard enough I will find some examples with 
LiveData
.
Don’t get me wrong, I understand all the interop support. But it makes everything so hard sometimes. 😔
a
flow has a lot of the same considerations that rxjava does in that there are many possible ways to say something depending on the characteristics you want the statement to have. LiveData took the approach of, "there's one way to do this and here it is," and frankly I don't think it aged very well as a result. Two examples of this are "cold" LiveData where 
.getValue()
always returns 
null
if no one ever subscribes, and the entire series of hacks and kludges around people trying to use LiveData for events.
these things aren't unique to flow+compose, they're for any uses of observability frameworks in general. But I totally agree that trying to learn them all at once is really overwhelming.
A lot of our docs right now are focused on the interop use cases but I think we have some things in store for more green-field compose development that will be a bit more simplified and prescriptive, without the constraints of meeting diverse existing codebases where they already are
❤️ 1
e
Do you have any rule of thumb for them? Basically I try to observe a 
sealed class
representing all the possible states from my View. It may or may not receive a parameter (like load all tasks with category id) and may or may not be a Flow in the database.
a
that's a very open-ended question 🙂
➕ 1
I tend to start by asking myself, what do I have, and what do I need?
If I have an idea for a screen or a widget on a screen, I ask what data I need to be able to render it
what are the valid states of that data, and how does the widget need to interact with that data? is there any feedback based on user actions? clicks? gestures?
If I can keep the widget as simple as 
@Composable fun MyWidget(value: ValueType, onValueChange: (ValueType) -> Unit)
then great; if it starts getting more complicated then I tend to start grouping those considerations into a hoisted state object of sorts
if that object's implementation is simple, it ends up being a final class. If it's complicated or has external dependencies that might be otherwise hard to test, or if it represents the wire-up of particular external dependencies, it often ends up being an interface
on the side of the data I'm working with, if the data source is hot, I tend to use 
by mutableStateOf(...)
properties. If the data source needs to know when something is observing it because it may be expensive to maintain ongoing updates otherwise, I tend to use 
Flow<T>
anything that happens over time is a 
suspend fun
- stay away from anything that is fire and forget, since it's hard to scope or cancel.
from there, compose is really good at scoping things like subscriptions or ongoing operations via suspend, and joining them together is usually quite pleasant. By extracting out more of the internal consistency management into state objects/interfaces, both the internal consistency management and the composables themselves become far easier to test, verify, and combine
e
Thanks again, Adam. I would love see more complex in the future! 😊 For sure it would help is a lot.
t
Thank you for this thread Adam, it's really helpful. I think I have a similar situation as Igor and removed repository fetching from the viewModel's 
init{}
. If I simply just need to fetch data and return a result without the need for any hot data, is it overkill to use the
Copy code
.stateIn(viewModelScope, SharingStarted.WhileSubscribed(), Unit)
operator for a flow in my viewmodel (which would then be collected in 
LaunchedEffect
? Seems like that way is still expecting hot data. Previously I was just listening to a 
StateFlow
for the result and then 
collectAsState
for the value in the composable. That way seemed a lot cleaner, and as long as I use 
LaunchedEffect
, I wouldn't have to worry about the flow still running, right?
a
This thread is a gold mine 😭. Thanks @Adam Powell. I've been executing initial usecases in my ViewModel.init{}, and been using MutableStateFlow for every viewmodel state. I need to get back to this thread and reflect on this when I get back to work.
👍 1
a
@Tony Kazanjian but what pushes into the 
StateFlow
you listen to? If there's a cold data source backing it you have to scope that somehow
t
It's just a usecase that fetches from our API with a lambda that returns a Result sealed class (with Success<T>, failure, loading states etc.). Maybe I'm confused about what is necessarily defined as hot vs cold data in this case. When Success or Failure is returned, I don't want to be observing the state anymore, basically
a
hot - updates happen regardless of whether or not they are being observed. cold - updates only happen when they are being observed.
t
Gotcha, so it sounds like from what you were saying before, despite whatever type of Flow we use, Compose will handle releasing the subscription as long as it's not in the viewModel init block. And if we only care about observing updates when the Composable is on screen, use a 
flow
instead of a 
StateFlow
, correct?
a
To a first approximation. Sharing makes things complicated. Isolated 1-1 producer-consumer cold subscriptions are easy. Shared 1-n producer-consumer hot subscriptions are also easy. It's when you want 1-n cold or n-m cold that things get complicated, and it's complicated because the problem is complicated; the APIs just reflect the problem complexity.
👍 2
b
You summed it up very well
e
Hello, again! Well, I spent the weekend thinking on everything that Adam shared (thanks again). At the moment, I’m trying to replace the 
StateFlow
and 
State
to cold 
Flow
and I’d like your opinions. 😊 A example of a simplified ViewModel:
Copy code
fun setTaskInfo(taskId: TaskId): Flow<TaskDetailState> = flow {
    val task = loadTaskUseCase(taskId = taskId.value)

    if (task != null) {
        emit(TaskDetailState.Loaded(task))
    } else {
        emit(TaskDetailState.Error)
    }
}

fun updateTitle(taskId: TaskId, title: String) {
    updateTaskTitle(taskId.value, title)
}
In the composable I simply:
Copy code
val detailViewState by detailViewModel
    .setTaskInfo(taskId = id)
    .collectAsState(initial = TaskDetailState.Loading)
Now basically all my “load data” functions returns a 
Flow
that is only observed during the Composable lifecycle. If something needs to be updated in the ViewModel, it passes the id again (onde it already has the reference). What do you think? 😬
a
Neat! You've also got a good place there to do any caching you might need in the future. Two things:
❤️ 1
Calling a method that returns a flow 
setFoo
is a bit strange, I'd try to find another name for it
❤️ 1
And since it returns a new flow on each invocation, you'll want to call it from your composable function in such a way that it remembers the flow and doesn't restart collection of a new flow on recomposition. e.g.
Copy code
val detailViewState by remember(detailViewModel, id) { detailViewModel.newName(id) }
  .collectAsState(initial = ...)
❤️ 1
👍 1
a
Btw the official compose samples are full of those stuff you recommend against 😞 ( launch in Viewmodel.init and use of MutableStateFlow everywhere). I guess that’s where I got the idea, and it’s also what felt natural to do (I overused PublishSubjects in RxJava too) I wish I could find examples or talks on these but it’s difficult to find.
e
Thanks a lot, Adam! ❤️ I think I’m starting get the concepts. 🥰
👍 1
a
I think we've got some new materials around this sort of thing in progress. A lot of the early ViewModel samples that use flows/coroutines were adapted from earlier LiveData samples, so some idioms carried over that don't necessarily translate directly. Some of the above is also my own opinions that are at varying stages of gathering consensus on the wider arch components team at the moment 🙂
👏 1
😮 1
e
After your comment about the 
remember
function for the 
Flow
, now the application seems great! Everything is called once and the state are working smoothly! 😍
👍 1
a
Sounds exciting! Looking forward to trying out these new materials.
e
If anyone is interested in the changes I did in my code based on Adam’s suggestions, this is the PR: https://github.com/igorescodro/alkaa/pull/133 😊
👍 2
s
I feel like there's a lot of info in here that should be a bit more widely known. I've been launching my flows on the 
init {}
of my ViewModels too. Is there no central source where all of this is a bit better explained just like we have this . I feel like so much good information gets hidden away inside these slack threads that 99.99% of the android devs won't ever see. (And I get FOMO and scroll slack too often too 😂)
a
There are some newer android+coroutines docs in the pipeline that should be published soon-ish. A number of us on the team and in the community in general tend to form opinions about things like this, talk about them, try them out in some code somewhere and see if they stand the test of time, it's good for things like this to simmer for a while before making it into docs that people will consider authoritative for years to come 🙂
👍 2
s
That's a fair point. I am happy that you are being careful with it. I am looking forward to read up on whatever does come out as soon as it's published!
j
Why would
Copy code
val detailViewState by remember(detailViewModel, id) { detailViewModel.newName(id) }
  .collectAsState(initial = ...)
Only collect once? Wouldn’t the 
collectAsState
get called on every recompose since it’s outside the remember block?
a
it remembers internally
There are some newer android+coroutines docs in the pipeline that should be published soon-ish
@Adam Powell were you talking about this article? https://developer.android.com/kotlin/coroutines/coroutines-best-practices It’s still using MutableStateFlows in the examples, and somehow manage to avoid talking about where or how the 
loadNews()
is supposed to be called 😄. It’s not specific to Compose and only in Compose it’s a bit vague how we’re supposed to execute the use cases.
a
hmm, I think I missed this one in the review because I flatly disagree w.r.t.: "The 
ViewModel
 class shouldn't expose suspend functions" 🙂
I agree with the premises that that section describes around considering scope of config changes, but I disagree with the conclusion. Fire and forget coroutines are an antipattern; in nearly all cases you want the signal of, "I don't care about the result anymore" that comes from coroutine cancellation to be available, even if you don't act on it by immediately cancelling the underlying operation.
exposing suspend functions for things like that instead of fire and forget requests make testing far easier, as you also know exactly when the operation is complete by way of the suspend function returning. You don't have to write tests that await side effects of the operation's completion instead, which get extra problematic if the fire and forget request ends up being idempotent.
The problem is visible in the example on the site: if a request results in an equal result being assigned to 
uiState
because the operation completed successfully but the result was the same, then you're asking your test to prove a negative, that an unwanted change will never come. How long do you wait to determine, "good enough" for that?
I'll follow up with the relevant folks later today
a
Looking forward to the improved docs later! 😄 I have questions about the article too, feel free to comment if you like https://kotlinlang.slack.com/archives/C0B8M7BUY/p1614873728315400?thread_ts=1614873728.315400&amp;cid=C0B8M7BUY
a
yeah, I saw that thread, I was waiting for Manuel to take first crack at it if he's around 🙂
m
Hope you don’t mind me jumping into this thread 🙂 Most of these decisions and best practices come from years of experience and looking at how the community do things. The need for a stream of 
UiState
that’s exposed from the ViewModel comes from the very need of surviving configuration changes and avoid inconsistent UI states. Most experienced developers have been fighting with this for a long time already. This is why patterns like MVI have gained so much weight over the past years. Obviously, we’ll need to see how all of this evolves with Compose and whether the current practices make sense anymore. Also, it’s difficult to come up with recommendations because which kind of developer should we target? Beginners? The average developer? or people working on the Android Toolkit developers? IMO, having an 
init
block inside a ViewModel whose result you expose using a 
StateFlow
to the UI is safer than exposing a suspend function to load data and let the UI decide what to do with it because sooo many things can go wrong.
❤️ 1
Said that, the discussion you had is very interesting. I agree that for a really simple use case like loading data, the 
tasksFlow
function looks great. But we’d also need to consider how that pattern scales when you have a much more complicated UI
😊 1
e
Thanks a lot for your thoughts, Manuel! I think the best part of all our discussion is that everyone were able to share their thoughts and create the best solution together. ❤️ Solutions to more complex problems takes time, but I’m very happy that you are listening to our feedbacks and acting fast. The support we have for a framework that is so new is awesome! Thank you all! 😊
I tried to summarize our discussion in an article published a couple of weeks ago. Today I updated it linking to the article you wrote yesterday. 🙂
❤️ 1
m
Sure! It’s great to have these conversations and learn from developer needs 🙂 with Compose specifically, we (DevRel) are also experimenting and trying to come up with what we think is best given the current state of the Android community. But as many of you, we’re constantly learning and adopting to the new technologies and patterns 🙂
❤️ 2
7 Views
Open in Slack
PreviousNext
Powered by