Skip to content

Refactor | Added extension functions#454

Closed
LinX64 wants to merge 9 commits intoandroid:mainfrom
LinX64:refactor_extension_function
Closed

Refactor | Added extension functions#454
LinX64 wants to merge 9 commits intoandroid:mainfrom
LinX64:refactor_extension_function

Conversation

@LinX64
Copy link
Copy Markdown
Contributor

@LinX64 LinX64 commented Nov 20, 2022

This PR is about polishing codes, and using extension functions for StateIn() (mostly in ViewModels).

Basically, all of the StateIn() functions inside ViewModels were following/using the same pattern, so I decided to create two extension functions for use in both ViewModels using viewModelScope, as well as CoroutineScope outside of ViewModels.

…ViewModels

This commit also includes refactoring for functions, making functions more clear by extracting them inside ViewModels.
There are two extension functions in this file, one is for ViewModels, and another one for using within a CoroutineScope.
* with the [initialValue].
*/
fun <T> Flow<T>.stateInCoroutineScope(
viewModelScope: CoroutineScope,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
viewModelScope: CoroutineScope,
coroutineScope: CoroutineScope,

Comment thread core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/Extenstions.kt Outdated
Removed the other extension, and used it with different configuration everywhere.
This shouldn't be starting with a capital keyword, it's an extension.
This was causing a leak of a job, and it was supposed to return Unit!
@LinX64 LinX64 requested a review from SimonMarquis November 26, 2022 12:54
@alexvanyo alexvanyo requested review from manuelvicnt and removed request for SimonMarquis November 28, 2022 21:18
@manuelvicnt
Copy link
Copy Markdown
Contributor

Unsure about adding this API / helper function to the Now in Android app.

It just saves you one line of code and the name (currently stateInScope) isn't clear what is doing or how it's different from the stateIn operator.

The ideal API for this would be a context receiver on both Flow and ViewModel so that you could have a Flow operator using the viewModelScope. e.g.

context(ViewModel)
fun Flow.produceState(
    coroutineScope: CoroutineScope = viewModelScope,
    started: SharingStarted = SharingStarted.WhileSubscribed(5_000),
    initialValue: T
): StateFlow<T> = ...

Until context receivers aren't widely available in Android and Compose, I'd refrain from adding a helper function for this functionality in a highly visible Android sample.

@LinX64
Copy link
Copy Markdown
Contributor Author

LinX64 commented Nov 29, 2022

Closing this PR as it is suggested to use Context receivers as a solution/replacement for the implementation.

P.S: This PR can wait for the Context Receivers to be available widely on Android and Kotlin.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants