Skip to content

ViewModel Scope #35

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 0 additions & 18 deletions app/src/main/kotlin/com/adesso/movee/base/BaseAndroidViewModel.kt
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,7 @@ import com.adesso.movee.internal.popup.PopupUiModel
import com.adesso.movee.internal.util.Event
import com.adesso.movee.internal.util.Failure
import com.adesso.movee.navigation.NavigationCommand
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.withContext

@Suppress("ConvertSecondaryConstructorToPrimary")
@SuppressLint("StaticFieldLeak")
Expand All @@ -34,9 +31,6 @@ abstract class BaseAndroidViewModel(application: Application) : AndroidViewModel

private val viewModelJob = SupervisorJob()

protected val uiScope = CoroutineScope(Dispatchers.Main + viewModelJob)
protected val bgScope = CoroutineScope(Dispatchers.Default + viewModelJob)

protected open fun handleFailure(failure: Failure) {
val (title, message) = when (failure) {
is Failure.NoConnectivityError -> Pair(
Expand Down Expand Up @@ -100,18 +94,6 @@ abstract class BaseAndroidViewModel(application: Application) : AndroidViewModel
_navigation.value = Event(NavigationCommand.Back)
}

protected suspend fun onUIThread(block: suspend CoroutineScope.() -> Unit) {
withContext(uiScope.coroutineContext) {
block.invoke(this)
}
}

protected suspend fun <T> onBackgroundThread(block: suspend CoroutineScope.() -> T): T {
return withContext(bgScope.coroutineContext) {
block.invoke(this)
}
}

protected fun getString(@StringRes resId: Int): String {
return getApplication<Application>().getString(resId)
}
Expand Down
14 changes: 13 additions & 1 deletion app/src/main/kotlin/com/adesso/movee/internal/util/UseCase.kt
Original file line number Diff line number Diff line change
@@ -1,19 +1,31 @@
package com.adesso.movee.internal.util

import com.adesso.movee.internal.util.functional.Either
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.async
import kotlinx.coroutines.launch

abstract class UseCase<out Type, in Params> where Type : Any {

protected abstract suspend fun buildUseCase(params: Params): Type

suspend fun run(params: Params): Either<Failure, Type> {
private suspend fun run(params: Params): Either<Failure, Type> {
return try {
Either.Right(buildUseCase(params))
} catch (failure: Failure) {
Either.Left(failure)
}
}

open operator fun invoke(
scope: CoroutineScope,
params: Params,
onResult: (Either<Failure, Type>) -> Unit = {}
) {
val backgroundJob = scope.async { run(params) }
scope.launch { onResult(backgroundJob.await()) }
Comment on lines +25 to +26
Copy link

@titanwalking titanwalking Apr 20, 2021

Choose a reason for hiding this comment

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

do we need to use async{} here? Wouldn't

        scope.launch {
            val result = run(params)
            onResult(result)
        }

suffice?

}

object None {
override fun toString() = "UseCase.None"
}
Expand Down
18 changes: 6 additions & 12 deletions app/src/main/kotlin/com/adesso/movee/scene/login/LoginViewModel.kt
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ import android.app.Application
import android.net.Uri
import androidx.lifecycle.LiveData
import androidx.lifecycle.MutableLiveData
import androidx.lifecycle.viewModelScope
import com.adesso.movee.base.BaseAndroidViewModel
import com.adesso.movee.domain.LoginUseCase
import com.adesso.movee.internal.util.Event
import javax.inject.Inject
import kotlinx.coroutines.launch

class LoginViewModel @Inject constructor(
private val loginUseCase: LoginUseCase,
Expand All @@ -28,19 +28,13 @@ class LoginViewModel @Inject constructor(
}

fun onLoginClick() {
uiScope.launch {
val username = username.value ?: return@launch
val password = password.value ?: return@launch

_loginInProgress.value = true

val loginResult = onBackgroundThread {
loginUseCase.run(LoginUseCase.Params(username, password))
}

loginResult.either(::handleFailure, ::navigateHome)
_loginInProgress.value = true

val username = username.value ?: return
val password = password.value ?: return
Comment on lines +31 to +34

Choose a reason for hiding this comment

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

line 31 should be below line 33 & 34 since one of them could return from the function and _loginInProgress.value would hang with true

loginUseCase.invoke(viewModelScope, LoginUseCase.Params(username, password)) {
_loginInProgress.value = false
it.either(::handleFailure, ::navigateHome)
}
}

Expand Down
18 changes: 5 additions & 13 deletions app/src/main/kotlin/com/adesso/movee/scene/movie/MovieViewModel.kt
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import android.app.Application
import androidx.annotation.StringRes
import androidx.lifecycle.LiveData
import androidx.lifecycle.MutableLiveData
import androidx.lifecycle.viewModelScope
import com.adesso.movee.R
import com.adesso.movee.base.BaseAndroidViewModel
import com.adesso.movee.domain.FetchNowPlayingMoviesUseCase
Expand All @@ -18,7 +19,6 @@ import com.adesso.movee.uimodel.MovieUiModel
import com.adesso.movee.uimodel.ShowHeaderUiModel
import com.adesso.movee.uimodel.ShowUiModel
import javax.inject.Inject
import kotlinx.coroutines.launch

class MovieViewModel @Inject constructor(
private val fetchPopularMoviesUseCase: FetchPopularMoviesUseCase,
Expand Down Expand Up @@ -49,12 +49,8 @@ class MovieViewModel @Inject constructor(
}

private fun fetchNowPlayingMovies() {
bgScope.launch {
val nowPlayingMoviesResult = fetchNowPlayingMoviesUseCase.run(UseCase.None)

onUIThread {
nowPlayingMoviesResult.either(::handleFailure, ::postNowPlayingMovieList)
}
fetchNowPlayingMoviesUseCase.invoke(viewModelScope, UseCase.None) {
it.either(::handleFailure, ::postNowPlayingMovieList)

Choose a reason for hiding this comment

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

postNowPlayingMovieList() uses LiveData's setValue() function which must be called from main thread. Either we need to use postValue(), or switch context when calling postNowPlayingMovieList()

}
}

Expand All @@ -63,12 +59,8 @@ class MovieViewModel @Inject constructor(
}

private fun fetchPopularMovies() {
bgScope.launch {
val popularMoviesResult = fetchPopularMoviesUseCase.run(UseCase.None)

onUIThread {
popularMoviesResult.either(::handleFailure, ::postPopularMovieList)
}
fetchPopularMoviesUseCase.invoke(viewModelScope, UseCase.None) {
it.either(::handleFailure, ::postPopularMovieList)

Choose a reason for hiding this comment

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

postPopularMovieList() uses LiveData's setValue() function which must be called from main thread. Either we need to use postValue(), or switch context when calling postPopularMovieList()

}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ package com.adesso.movee.scene.moviedetail
import android.app.Application
import androidx.lifecycle.LiveData
import androidx.lifecycle.MutableLiveData
import androidx.lifecycle.viewModelScope
import com.adesso.movee.base.BaseAndroidViewModel
import com.adesso.movee.domain.FetchMovieCreditsUseCase
import com.adesso.movee.domain.FetchMovieDetailUseCase
import com.adesso.movee.uimodel.MovieCreditUiModel
import com.adesso.movee.uimodel.MovieDetailUiModel
import javax.inject.Inject
import kotlinx.coroutines.launch

class MovieDetailViewModel @Inject constructor(
private val fetchMovieDetailUseCase: FetchMovieDetailUseCase,
Expand All @@ -24,13 +24,8 @@ class MovieDetailViewModel @Inject constructor(

fun fetchMovieDetails(id: Long) {
if (_movieDetails.value == null) {
bgScope.launch {
val movieDetailResult =
fetchMovieDetailUseCase.run(FetchMovieDetailUseCase.Params(id))

onUIThread {
movieDetailResult.either(::handleFailure, ::postMovieDetail)
}
fetchMovieDetailUseCase.invoke(viewModelScope, FetchMovieDetailUseCase.Params(id)) {
it.either(::handleFailure, ::postMovieDetail)

Choose a reason for hiding this comment

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

postMovieDetail() uses LiveData's setValue() function which must be called from main thread. Either we need to use postValue(), or switch context when calling postMovieDetail()

}
}
}
Expand All @@ -41,13 +36,8 @@ class MovieDetailViewModel @Inject constructor(

fun fetchMovieCredits(id: Long) {
if (_movieCredits.value == null) {
bgScope.launch {
val movieCreditsResult =
fetchMovieCreditsUseCase.run(FetchMovieCreditsUseCase.Params(id))

onUIThread {
movieCreditsResult.either(::handleFailure, ::postMovieCredits)
}
fetchMovieCreditsUseCase.invoke(viewModelScope, FetchMovieCreditsUseCase.Params(id)) {
it.either(::handleFailure, ::postMovieCredits)

Choose a reason for hiding this comment

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

postMovieCredits() uses LiveData's setValue() function which must be called from main thread. Either we need to use postValue(), or switch context when calling postMovieCredits()

}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package com.adesso.movee.scene.persondetail
import android.app.Application
import androidx.lifecycle.LiveData
import androidx.lifecycle.MutableLiveData
import androidx.lifecycle.viewModelScope
import com.adesso.movee.base.BaseAndroidViewModel
import com.adesso.movee.domain.FetchPersonDetailsUseCase
import com.adesso.movee.internal.util.AppBarStateChangeListener
Expand All @@ -11,7 +12,6 @@ import com.adesso.movee.internal.util.AppBarStateChangeListener.State.EXPANDED
import com.adesso.movee.internal.util.AppBarStateChangeListener.State.IDLE
import com.adesso.movee.uimodel.PersonDetailUiModel
import javax.inject.Inject
import kotlinx.coroutines.launch

class PersonDetailViewModel @Inject constructor(
private val fetchPersonDetailsUseCase: FetchPersonDetailsUseCase,
Expand All @@ -25,13 +25,11 @@ class PersonDetailViewModel @Inject constructor(

fun fetchPersonDetails(personId: Long) {
if (_personDetails.value == null) {
bgScope.launch {
val personDetailResult =
fetchPersonDetailsUseCase.run(FetchPersonDetailsUseCase.Params(personId))

onUIThread {
personDetailResult.either(::handleFailure, ::postPersonDetails)
}
fetchPersonDetailsUseCase.invoke(
viewModelScope,
FetchPersonDetailsUseCase.Params(personId)
) {
it.either(::handleFailure, ::postPersonDetails)

Choose a reason for hiding this comment

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

postPersonDetails() uses LiveData's setValue() function which must be called from main thread. Either we need to use postValue(), or switch context when calling postPersonDetails()

}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@ import android.app.Application
import androidx.lifecycle.LiveData
import androidx.lifecycle.MutableLiveData
import androidx.lifecycle.Transformations
import androidx.lifecycle.viewModelScope
import com.adesso.movee.base.BaseAndroidViewModel
import com.adesso.movee.domain.FetchUserDetailsUseCase
import com.adesso.movee.domain.GetLoginStateUseCase
import com.adesso.movee.internal.util.UseCase
import com.adesso.movee.uimodel.LoginState
import com.adesso.movee.uimodel.UserDetailUiModel
import javax.inject.Inject
import kotlinx.coroutines.launch

class ProfileViewModel @Inject constructor(
private val fetchUserDetailsUseCase: FetchUserDetailsUseCase,
Expand All @@ -32,12 +32,8 @@ class ProfileViewModel @Inject constructor(
}

private fun getLoginState() {
bgScope.launch {
val loginStateResult = getLoginStateUseCase.run(UseCase.None)

onUIThread {
loginStateResult.either(::handleFailure, ::handleLoginStateSuccess)
}
getLoginStateUseCase.invoke(viewModelScope, UseCase.None) {
it.either(::handleFailure, ::handleLoginStateSuccess)

Choose a reason for hiding this comment

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

handleLoginStateSuccess() calls postLoginState() inside. postLoginState() uses LiveData's setValue() function which must be called from main thread. Either we need to use postValue(), or switch context when calling postLoginState(). Also, handleLoginStateSuccess() calls updateUserDetails() which is a network operation and switches context with Dispatchers.IO. Using postValue() at postLoginState() would better suit here.

}
}

Expand All @@ -58,12 +54,8 @@ class ProfileViewModel @Inject constructor(
}

private fun fetchUserDetails() {
bgScope.launch {
val userDetailsResult = fetchUserDetailsUseCase.run(UseCase.None)

onUIThread {
userDetailsResult.either(::handleFailure, ::postUserDetails)
}
fetchUserDetailsUseCase.invoke(viewModelScope, UseCase.None) {
it.either(::handleFailure, ::postUserDetails)

Choose a reason for hiding this comment

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

postUserDetails() uses LiveData's setValue() function which must be called from main thread. Either we need to use postValue(), or switch context when calling postUserDetails()

}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package com.adesso.movee.scene.search
import android.app.Application
import androidx.lifecycle.LiveData
import androidx.lifecycle.MutableLiveData
import androidx.lifecycle.viewModelScope
import com.adesso.movee.base.BaseAndroidViewModel
import com.adesso.movee.domain.MultiSearchUseCase
import com.adesso.movee.internal.util.Failure
Expand All @@ -13,7 +14,6 @@ import com.adesso.movee.uimodel.TvShowMultiSearchUiModel
import javax.inject.Inject
import kotlinx.coroutines.CancellationException
import kotlinx.coroutines.Job
import kotlinx.coroutines.launch

class SearchViewModel @Inject constructor(
private val multiSearchUseCase: MultiSearchUseCase,
Expand All @@ -31,11 +31,12 @@ class SearchViewModel @Inject constructor(
val query = text?.trim() ?: return
if (query.length > MIN_SEARCHABLE_LENGTH) {
multiSearchJob?.cancel()
multiSearchJob = bgScope.launch {
val searchResult = multiSearchUseCase.run(MultiSearchUseCase.Params(query))
onUIThread {
searchResult.either(::handleSearchFailure, ::postMultiSearchResult)
}

multiSearchUseCase.invoke(
viewModelScope,
MultiSearchUseCase.Params(query)
) {
it.either(::handleSearchFailure, ::postMultiSearchResult)

Choose a reason for hiding this comment

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

postMultiSearchResult() uses LiveData's setValue() functions which must be called from main thread. Either we need to use postValue() on both LiveDatas we wan't their values to change, or switch context when calling postMultiSearchResult()

}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package com.adesso.movee.scene.tvshow
import android.app.Application
import androidx.lifecycle.LiveData
import androidx.lifecycle.MutableLiveData
import androidx.lifecycle.viewModelScope
import com.adesso.movee.R
import com.adesso.movee.base.BaseAndroidViewModel
import com.adesso.movee.domain.FetchNowPlayingTvShowsUseCase
Expand All @@ -17,7 +18,6 @@ import com.adesso.movee.uimodel.ShowHeaderUiModel
import com.adesso.movee.uimodel.ShowUiModel
import com.adesso.movee.uimodel.TvShowUiModel
import javax.inject.Inject
import kotlinx.coroutines.launch

class TvShowViewModel @Inject constructor(
private val fetchTopRatedTvShowsUseCase: FetchTopRatedTvShowsUseCase,
Expand Down Expand Up @@ -48,12 +48,8 @@ class TvShowViewModel @Inject constructor(
}

private fun fetchTopRatedTvShows() {
bgScope.launch {
val topRatedTvShowsResult = fetchTopRatedTvShowsUseCase.run(UseCase.None)

onUIThread {
topRatedTvShowsResult.either(::handleFailure, ::postTopRatedTvShows)
}
fetchTopRatedTvShowsUseCase.invoke(viewModelScope, UseCase.None) {
it.either(::handleFailure, ::postTopRatedTvShows)

Choose a reason for hiding this comment

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

postTopRatedTvShows() uses LiveData's setValue() function which must be called from main thread. Either we need to use postValue(), or switch context when calling postTopRatedTvShows()

}
}

Expand All @@ -62,12 +58,8 @@ class TvShowViewModel @Inject constructor(
}

private fun fetchNowPlayingTvShows() {
bgScope.launch {
val nowPlayingTvShowsResult = fetchNowPlayingTvShowsUseCase.run(UseCase.None)

onUIThread {
nowPlayingTvShowsResult.either(::handleFailure, ::postNowPlayingTvShows)
}
fetchNowPlayingTvShowsUseCase.invoke(viewModelScope, UseCase.None) {
it.either(::handleFailure, ::postNowPlayingTvShows)

Choose a reason for hiding this comment

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

postNowPlayingTvShows() uses LiveData's setValue() function which must be called from main thread. Either we need to use postValue(), or switch context when calling postNowPlayingTvShows()

}
}

Expand Down
Loading