Kotlin / kotlinx.coroutines

Library support for Kotlin coroutines
Apache License 2.0
12.94k stars 1.84k forks source link

Initialize a MutableStateFlow without an initial value #2515

Open andreimesina opened 3 years ago

andreimesina commented 3 years ago

Can we have a way of initializing a MutableStateFlow without having to use an initial value for it?

Quoting Kotlin official docs:

Conceptually, state flow is similar to ConflatedBroadcastChannel and is designed to completely replace ConflatedBroadcastChannel in the future. To migrate ConflatedBroadcastChannel usage to StateFlow, start by replacing usages of the ConflatedBroadcastChannel() constructor with MutableStateFlow(initialValue), using null as an initial value if you don’t have one.

There are many cases where an object cannot really have a default "empty" implementation, and having to make the T in StateFlow<T> type nullable in order to support lazy initialization of objects might also introduce some bad consequences to the code base...

Could we rather have a default MutableStateFlow() constructor, combined with throwing an exception if stateFlow.value is accessed without having any value? This maybe belongs to the fail-safe vs fail-fast topic, but I really wish we did not have to make the T type nullable.

psteiger commented 3 years ago

Alternatively you can use SharedFlow created with replay=1, onBufferOverflow= BufferOverflow.DROP_OLDEST and apply distinctUntilChanged() to make it behave identically to a StateFlow, except it won't require an initial value.

Value can be accessed through first() if really required.

See https://kotlin.github.io/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines.flow/-state-flow/

andreimesina commented 3 years ago

Hi @psteiger , thanks for the reply.

That is a solution, but it would also be really cool if we could have it straight wrapped up under a MutableStateFlow() constructor, just like ConflatedBroadcastChannel does.

Creating a SharedFlow with the aforementioned parameters would work but it would most probably require extra explanation during the code review process on the projects for anyone that is not yet aware of this option. It also makes the implementation a bit vulnerable to breaking API changes for those SharedFlow parameters as I imagine.

So I think my question still stands on a good principle: can we have a MutableStateFlow() constructor that takes no initial value? It would definitely benefit all the Coroutines consumers.

psteiger commented 3 years ago

Hi @andreimesina ,

I understand your points and I can say I also missed the no-initial-value StateFlow.

But I also think that your suggestion of making .value access raise an exception can cause so much unexpected errors in user code. Basically we would never be sure anymore if accessing a StateFlow value would be safe, and we would need to wrap every .value access in a try catch of some kind. I don't think it's worth it.

Have you considered wrapping every value in a "Resource" sealed class? Then all your StateFlow lacking a proper initial value could be constructed with Resource.Empty instead of null, if you are so averse to null :).

I myself usually prefer just using SharedFlow configured similarly to a StateFlow. Note that a StateFlow is a SharedFlow. A constricted, optimized SharedFlow. SharedFlow is very powerful and flexible, I only ever use StateFlow if it fits very well the use case, and if I don't have a natural initial value, then it does not fit the use case.

P.S. in case you are not aware, this has been discussed before on other issues on this repository, you might want to check the history.

andreimesina commented 3 years ago

Hey @psteiger , I could not find any other open/closed issue that points to this specific request on this repo unfortunately.

I agree that having to wrap it in a try - catch expression every time would be bad, that's why I liked the ConflatedBroadcastChannel more as it was flexible - it offers both value and valueOrNull. And you can decide based on your use case if you will have it nullable or if you will just catch the exception.

A Resource sealed class would imply a high refactoring job in very big projects and it would also force all existing / new Resource classes to stop inheriting from another class if needed.

The SharedFlow configured as a StateFlow will probably be the way to go, but it still feels incomplete as we would have to create our own mutableShareFlow() initializer method so we don't need to repeat replay = 1, onBufferOverflow = DROP_OLDEST every time we declare one...

superuserdid commented 3 years ago

Might I add here that using first() is also somewhat a code smell as first() belongs to the Flow class and triggers the entire upstream Flow and returns the first result. Changing from any of the Flow subclasses wouldn't change or notify any downstream consumers, which could have really significant unintended effects. Most of the time it seems first() is used, you really should consider using a StateFlow as you really want the current value from the Flow, and not to trigger the entire flow again only to return the first result. I too have found myself in a situation where creating a StateFlow is desired without an initial value. Having a sealed class is useful based on use case, but for components that are defined in lower parts of your architecture, it doesn't make much sense and also requires all consumers to use a when statement or similar to access what they need when they need. We do have the ability to use stateIn() but this requires a suspend function to use which is not as flexible as shareIn(). This would be great if we could use.

pitoszud commented 3 years ago

You can use return@collect in the collect lambda block if you don't want to add a null check for your consumed object.

val flowItems = myRepo.getMyFlow().stateIn(viewModelScope, SharingStarted.Lazily, null)
viewModel.flowItems.collect { val flowItems = it ?: return@collect }

Alternatively You can also use viewModel.flowItems.collect { it?.let { flowItems -> }

For local/remote use cases you can use a Resource sealed class, with a loading state as an initial value. val flowItems = myRepo.getMyFlow().stateIn(viewModelScope, SharingStarted.Lazily, Resource.Loading)

rinatdoter commented 3 years ago

Hi @andreimesina ,

I understand your points and I can say I also missed the no-initial-value StateFlow.

But I also think that your suggestion of making .value access raise an exception can cause so much unexpected errors in user code. Basically we would never be sure anymore if accessing a StateFlow value would be safe, and we would need to wrap every .value access in a try catch of some kind. I don't think it's worth it.

Have you considered wrapping every value in a "Resource" sealed class? Then all your StateFlow lacking a proper initial value could be constructed with Resource.Empty instead of null, if you are so averse to null :).

I myself usually prefer just using SharedFlow configured similarly to a StateFlow. Note that a StateFlow is a SharedFlow. A constricted, optimized SharedFlow. SharedFlow is very powerful and flexible, I only ever use StateFlow if it fits very well the use case, and if I don't have a natural initial value, then it does not fit the use case.

P.S. in case you are not aware, this has been discussed before on other issues on this repository, you might want to check the history.

LiveData doesn't require initial value. It just returns null, when accessed through livedata.value. Why can't this be case with StateFlow?

caiodev commented 3 years ago

@rinatdoter You're right. What I'm doing as a workaround is to set an initial value that is not used. For example:

private val _successStateFlow = MutableStateFlow<State>(Initial)

when(value) { Initial -> Unit //It is mapped but does nothing }

So when the default value is posted, it won't interfere in my execution flow

elizarov commented 3 years ago

LiveData doesn't require initial value. It just returns null, when accessed through livedata.value. Why can't this be case with StateFlow?

@rinatdoter Just use MutableStateFlow<MyType?>(null) and you can use it just like LiveData — with null initial value.

andreimesina commented 3 years ago

LiveData doesn't require initial value. It just returns null, when accessed through livedata.value. Why can't this be case with StateFlow?

@rinatdoter Just use MutableStateFlow<MyType?>(null) and you can use it just like LiveData — with null initial value.

@elizarov What about the case where you specifically need to treat the null case for a field? This is a very common situation and there is no easy Flow first-party solution for this, you either need to wrap the fields in a Resource type which I have already mentioned is inefficient to do when migrating big apps, or you need to have some kind of isFirstCollect = true in order to skip the first null collection on those particular flows, which is definitely a code smell...

pitoszud commented 2 years ago

Most of the time it seems first() is used, you really should consider using a StateFlow as you really want the current value from the Flow, and not to trigger the entire flow again only to return the first result.

Just to clarify @superuserdid response first() is a terminal operator that returns the first element emitted by the flow, so it will basically cancel the flow's collection. It can be called on the StateFlow as it inherits the function from Flow class. Please note, it has to be called from a coroutine or suspend function. If you want a current state of the StateFlow without terminating the flow and without getting the state from the suspend function, use StateFlow.value.

Djambulat69 commented 2 years ago

drop(1)