-
Notifications
You must be signed in to change notification settings - Fork 241
coroutines homework #261
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
base: development
Are you sure you want to change the base?
coroutines homework #261
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,28 +1,82 @@ | ||
| package otus.homework.coroutines | ||
|
|
||
| import retrofit2.Call | ||
| import retrofit2.Callback | ||
| import retrofit2.Response | ||
| import kotlinx.coroutines.CancellationException | ||
| import kotlinx.coroutines.CoroutineName | ||
| import kotlinx.coroutines.CoroutineScope | ||
| import kotlinx.coroutines.Dispatchers | ||
| import kotlinx.coroutines.SupervisorJob | ||
| import kotlinx.coroutines.async | ||
| import kotlinx.coroutines.cancelChildren | ||
| import kotlinx.coroutines.launch | ||
| import kotlinx.coroutines.withContext | ||
| import java.net.SocketTimeoutException | ||
|
|
||
| class CatsPresenter( | ||
| private val catsService: CatsService | ||
| ) { | ||
|
|
||
| private val scope = CoroutineScope( | ||
| Dispatchers.Main.immediate + SupervisorJob() + CoroutineName("CatsCoroutine") | ||
| ) | ||
| private var _catsView: ICatsView? = null | ||
|
|
||
|
|
||
| /** | ||
| * Не понял по условию задачи, что нужно делать, если в одном из запросов ошибка, | ||
| * а вдругом нет и на чьи ошибки надо показывать тосты. Тут тосты показываются только на ошибки | ||
| * в запросе фактов, и ошибка в одном из запросов не отменяет другой | ||
| * | ||
| * Во вьюмоделе сделаю, чтоб ошибка в любом из запросов отменяет оба запроса и ошибки из любого | ||
| * выводятся в текстовое поле | ||
| */ | ||
| fun onInitComplete() { | ||
| catsService.getCatFact().enqueue(object : Callback<Fact> { | ||
| scope.coroutineContext.cancelChildren() | ||
| val factJob = scope.async(Dispatchers.IO) { | ||
| try { | ||
| val response = catsService.getCatFact() | ||
| if (response.isSuccessful) { | ||
| response.body() | ||
| } else { | ||
| null | ||
| } | ||
| } catch (e: Throwable) { | ||
| if (e is CancellationException) throw e | ||
| when (e) { | ||
| is SocketTimeoutException -> { | ||
| withContext(Dispatchers.Main) { | ||
| _catsView?.showToast("Не удалось получить ответ от сервера") | ||
| } | ||
| } | ||
|
|
||
| override fun onResponse(call: Call<Fact>, response: Response<Fact>) { | ||
| if (response.isSuccessful && response.body() != null) { | ||
| _catsView?.populate(response.body()!!) | ||
| else -> { | ||
| CrashMonitor.trackWarning() | ||
| withContext(Dispatchers.Main) { | ||
| _catsView?.showToast(e.message) | ||
| } | ||
| } | ||
| } | ||
| null | ||
| } | ||
| } | ||
|
|
||
| override fun onFailure(call: Call<Fact>, t: Throwable) { | ||
| CrashMonitor.trackWarning() | ||
| val imageJob = scope.async(Dispatchers.IO) { | ||
| try { | ||
| val response = catsService.getRandomImage() | ||
| if (response.isSuccessful) { | ||
| response.body()?.firstOrNull() | ||
| } else { | ||
| null | ||
| } | ||
| } catch (e: Throwable) { | ||
| if (e is CancellationException) throw e | ||
| null | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| scope.launch { | ||
| val fact = factJob.await() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. тут не выполнено требование одновременной отмены запросов, и launch и оба async запущены на одной SupervisorJob и отмена одной загрузки никак не повлияет на другую
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Там нет такого требования там написано: "Отменятся запросы должны одновременно". Там не написано отменять оба запроса в случае ошибки в любом из них. Отменяю я запросы при закрытии экрана вместе с гибелью скоупа. Это происходит одновременно |
||
| val image = imageJob.await() | ||
| _catsView?.populate(CatsUiModel(fact?.fact, image?.url)) | ||
| } | ||
| } | ||
|
|
||
| fun attachView(catsView: ICatsView) { | ||
|
|
@@ -31,5 +85,6 @@ class CatsPresenter( | |
|
|
||
| fun detachView() { | ||
| _catsView = null | ||
| scope.coroutineContext.cancelChildren() | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,15 @@ | ||
| package otus.homework.coroutines | ||
|
|
||
| import retrofit2.Call | ||
| import retrofit2.Response | ||
| import retrofit2.http.GET | ||
|
|
||
| interface CatsService { | ||
|
|
||
| @GET("fact") | ||
| fun getCatFact() : Call<Fact> | ||
| suspend fun getCatFact(): Response<Fact> | ||
|
|
||
| @GET("https://api.thecatapi.com/v1/images/search") | ||
| suspend fun getRandomImage(): Response<List<Image>> | ||
|
|
||
|
|
||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| package otus.homework.coroutines | ||
|
|
||
| data class CatsUiModel(val fact: String? = null, val image: String? = null) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| package otus.homework.coroutines | ||
|
|
||
| import androidx.lifecycle.ViewModel | ||
| import androidx.lifecycle.ViewModelProvider | ||
| import androidx.lifecycle.viewModelScope | ||
| import kotlinx.coroutines.CoroutineExceptionHandler | ||
| import kotlinx.coroutines.Dispatchers | ||
| import kotlinx.coroutines.async | ||
| import kotlinx.coroutines.cancelChildren | ||
| import kotlinx.coroutines.flow.MutableStateFlow | ||
| import kotlinx.coroutines.flow.StateFlow | ||
| import kotlinx.coroutines.flow.asStateFlow | ||
| import kotlinx.coroutines.launch | ||
| import java.net.SocketTimeoutException | ||
|
|
||
| class CatsViewModel(private val catsService: CatsService) : ViewModel() { | ||
| private val _uiModelFlow = MutableStateFlow<Result>(Result.Loading) | ||
| val uiModelFlow: StateFlow<Result> = _uiModelFlow.asStateFlow() | ||
| private val exceptionHandler = CoroutineExceptionHandler { _, e -> | ||
| when (e) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. в exceptionHandler лучше не обрабатывать ошибки, а только делать логирование, что и указано в условиях задачи, сюда должны попасть неперехваченные в try-catch ошибки
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Тогда стэйт ошибки не дойдет никогда до ui. Либо внутри async ловить ошибки, тогда они не дойдут до exceptionHandler-а, чтобы там залогироваться. Либо давать им уходить в exceptionHandler и если там только логировать, то стэйт не обновиться. |
||
| is SocketTimeoutException -> { | ||
| _uiModelFlow.tryEmit(Result.Error(message = "Не удалось получить ответ от сервера")) | ||
| } | ||
|
|
||
| else -> { | ||
| CrashMonitor.trackWarning() | ||
| _uiModelFlow.tryEmit(Result.Error(message = e.message)) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| fun load() { | ||
| viewModelScope.coroutineContext.cancelChildren() | ||
| viewModelScope.launch(exceptionHandler) { | ||
| _uiModelFlow.emit(Result.Loading) | ||
| val factJob = async(Dispatchers.IO) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. хорошая практика передавать Dispatchers через конструктор вьюмодели, чтобы его можно было подменять (например в тестах)
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Не спорю, не было такой задачи |
||
| val response = catsService.getCatFact() | ||
| if (response.isSuccessful) { | ||
| response.body() | ||
| } else { | ||
| null | ||
| } | ||
| } | ||
|
|
||
| val imageJob = async(Dispatchers.IO) { | ||
| val response = catsService.getRandomImage() | ||
| if (response.isSuccessful) { | ||
| response.body()?.firstOrNull() | ||
| } else { | ||
| null | ||
| } | ||
| } | ||
|
|
||
| val factResult = factJob.await() | ||
| val imageResult = imageJob.await() | ||
| _uiModelFlow.emit( | ||
| Result.Success( | ||
| CatsUiModel( | ||
| fact = factResult?.fact, | ||
| image = imageResult?.url | ||
| ) | ||
| ) | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| class Factory(private val catsService: CatsService) : ViewModelProvider.Factory { | ||
| @Suppress("UNCHECKED_CAST") | ||
| override fun <T : ViewModel> create(modelClass: Class<T>): T { | ||
| return CatsViewModel(catsService) as T | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| package otus.homework.coroutines | ||
|
|
||
| import com.google.gson.annotations.SerializedName | ||
|
|
||
| data class Image( | ||
| @field:SerializedName("id") | ||
| val id: String, | ||
| @field:SerializedName("url") | ||
| val url: String, | ||
| @field:SerializedName("width") | ||
| val width: Int, | ||
| @field:SerializedName("height") | ||
| val height: Int, | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| package otus.homework.coroutines | ||
|
|
||
| sealed interface Result { | ||
| data class Success(val uiModel: CatsUiModel) : Result | ||
| data class Error(val message: String?) : Result | ||
| object Loading : Result | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. минорно, для удобства лучше сделать data object
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Только ради генерации toString()? Ну да, хуже не будет.) Но для этого надо котлин поднять в проекте, так что пожалуй не буду |
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
нет обработки SocketTimeoutException
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А там четко не сказано, что это надо для картинок и что вообще должно происходить, когда ошибка только в одном из запросов. Какое условие такой и результат