Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing StateFlow operators #2008

Closed
OrhanTozan opened this issue May 10, 2020 · 9 comments
Closed

Missing StateFlow operators #2008

OrhanTozan opened this issue May 10, 2020 · 9 comments

Comments

@OrhanTozan
Copy link

OrhanTozan commented May 10, 2020

I'm missing operators for StateFlow<T> such as map and transform. Example use case:

val name: StateFlow<String> = MutableStateFlow("Eric")
val greeting: StateFlow<String> = name.map { name -> "Hi $name!" }

This would be the same API Android's LiveData provides.

@Dominaezzz
Copy link
Contributor

I think using stateIn is the way to go here.
i.e. name.map { name -> "Hi $name!" }.stateIn(...)

That way you can do more transformations between the two StateFlows without the cumulative overhead (locks) in-between.
Also, redefining operators is somewhat counter-intuitive to one of the things Flows were intending to achieve, unifying stream operations (Like the ones from Channel which were deprecated).

@OrhanTozan
Copy link
Author

Where can I find more information about stateIn? Can't find it in the docs.

@fvasco
Copy link
Contributor

fvasco commented May 10, 2020

@NahroTo
the stateIn operator isn't available in the version 1.3.6.

@elizarov
Copy link
Contributor

The design of stateIn operator was posted for review in #2047 and, I believe, it fully covers the case of missing state flow operator. Any chain of operators on a state flow can be materialized into another StateFlow using stateIn operator, so I'm closing this issue. If anything is missing, please comment under #2047.

@brescia123
Copy link

@elizarov What if I want to simply transform with map an already available StateFlow into a new StateFlow? I will have to explicitly cast the return of map (that is Flow) to StateFlow?

@fvasco
Copy link
Contributor

fvasco commented Feb 23, 2021

@brescia123, updating the return type is a breaking change.
Finally, please consider that a StateFlow is more expensive than a Flow, it isn't a simple cast.

@neworld
Copy link

neworld commented Mar 30, 2021

Any chain of operators on a state flow can be materialized into another StateFlow using stateIn operator

We are on transit from LiveData to StateFlow, because it allows us to write more declarative code. It also means we are writing many chains. However, many state flows have stateIn in the end. Because fragment can't consume suspendable version of stateIn we always need to provide some dummy object for the state. It does not look optimal due to unnecessary memory allocation.

Just an idea, but maybe it is worth considering implement non-suspendable version of few operators, who guarantee the result will be correct. For example mapState and combineState cover almost all chains and should guarantee correctness

@elizarov
Copy link
Contributor

elizarov commented Apr 5, 2021

@neworld Can you, please, elaborate a bit on what you mean by "fragment can't consume suspendable version" and give some examples of the code you are having to write in your app.

@neworld
Copy link

neworld commented Apr 13, 2021

@elizarov, sorry for my long delay.

fragment can't consume suspendable version

This is a tricky part because fragment actually could consume suspend functions because it has scopes. But it is more UX/philosophical problem rather than a technical one. Fragments always have to show some state and a blank screen is not desired behavior. I didn't tried, but Jetpack Compose should not be compatible with suspended state (collectAsState).

But to be honest, I can't find any big problem in ViewModel to use stateIn with a default value. However, it causes some inconveniences:

class MyViewModel(val partiallyLoadedItem: Item): ViewModel {
    private val item: StateFlow<Item> = database.loadItem()
    val viewState: StateFlow<ViewState> = flowOf(partiallyLoadedItem).onCompletion { emitAll(realItem) }.map(::mapItemToState).collectIn(this, Lazy, ???)

    // but I could map two times:

    val viewState: StateFlow<ViewState> = realItem.map(::mapItemToState).collectIn(this, Lazy, mapItemToState(partiallyLoadedItem))
}

For longer chains, it is harder to follow, because the initial item is at the end of the chain.

But put ViewModel aside. The real problem we are facing is inside scopeless helper classes:

class ItemManagerImpl(): ItemManager, MutableItemManager {
    private val itemSelectionState = MutableStateFlow(ItemSelectionState())
    override val state = itemSelectionState.asStateFlow()

    override val validation = itemSelectionState.map(::validateSelection).stateIn(???)
}

What is a best practice here? Some ideas we have:

  • Add a scope to ItemManagerImpl and have liability to close the scope
  • Use suspend fun instead of property. The suspension point will boil down to the fragment and we need to deal with it anyway
  • Update both values inside function:
    @Synchronized
    override fun updateState(itemSelectionState: ItemSelectionState) {
        this.itemSelectionState.value = itemSelectionState
        _validation.value = itemSelectionValidator.validate(itemSelectionState)
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants