Задача по Coroutines #160

SergeyKozhukhov · 2023-07-16T17:35:56Z

No description provided.

- add custom view with custom ViewModelStoreOwner and LifecycleOwner - refactoring

SergeyKozhukhov · 2023-07-16T18:38:40Z

app/src/main/java/otus/homework/coroutines/models/presentation/CatUiState.kt

+     * @param isShown признак отображения причины отсутствия данных
+     */
+    data class Error(val message: String, val isShown: Boolean = false) : CatUiState()
+}


isShown нужен, чтобы при пересоздании activity заново не отображать Toast

Идея понятная, но если ты хочешь ее решить совсем правильно то тебе нужно 2 лайвдаты/стейтфлоу. Один который с синглевентами/реплеями а другой без

Сделал, для одного варианта с ViewModel использования StateFlow и Channel.
Но почему для правильности надо разбивать?? Насколько понимаю, наоборот подход через StateFlow корректный. В том числе на занятии по MVI его рассматривали.
Вот статья про подход
https://scribe.rip/androiddevelopers/viewmodel-one-off-event-antipatterns-16a1da869b95

SergeyKozhukhov · 2023-07-16T18:40:10Z

app/src/main/java/otus/homework/coroutines/MainActivity.kt

+                Intent(
+                    this, otus.homework.coroutines.presentation.mvvm.owners.CatsActivity::class.java
+                )
+            )


Activity ведет на другие три activity с тремя вариантами решения: с презентером, ViewModel и ViewModel с кастомными owаner-ами

SergeyKozhukhov · 2023-07-16T18:41:09Z

app/src/main/java/otus/homework/coroutines/di/DiContainer.kt

+
+    private companion object {
+        const val FACT_BASE_URL = "https://catfact.ninja/"
+        const val IMAGES_BASE_URL = "https://api.thecatapi.com/v1/images/"


Сайт, указанный в ДЗ не грузится. Взял другое API, которое нашел

SergeyKozhukhov · 2023-07-16T18:47:07Z

app/src/main/java/otus/homework/coroutines/presentation/mvp/CatsPresenter.kt

+                } else {
+                    CrashMonitor.trackWarning(e)
+                    _catsView?.warn(message = e.messageOrDefault())
+                }


Вот тут непонимание. Не пробрасываю дальше CancelationException, так как PresenterScope не supervisor. Если здесь придет ошибка, то не хотелось бы, чтобы скоп отменился. Хотелось бы, чтобы дальше могли делать запросы. Поэтому не пробрасываю CancelationException. Но верно ли так??? Все-таки ниже отменяю при detachView job. Но, наверное, тогда и отмена не произойдет, так как не пробрасывается CancelationException. С другой стороны detachView вызывается в onStop, а по возвращению бы все-таки иметь живой скоп, на котором запускать launch. Или все-таки что-то путаю. При пересоздана activity все-равно будет здесь создан новый скоп, можно было бы и скоп весь отменить. Но все равно, надо ли тут пробрасывать Cancelation...

Или может делать в detachView
scope.coroutineContext.cancelChildren()?

Если хочешь чтобы не отменялся то целевой механизм это супервайзор

Эксепшен лучше пробросить, либо написать catch на нужные типы исключений. Ну либо взять ексепшенхендлер

В том числе и поэтому спрашивал про ошибки ретрофита)
Пробросил здесь CancellationException.

Прочитал в этой статье про распространенную ошибку, что не пробрасывается дальше CancellationException.
https://www.lukaslechner.com/7-common-mistakes-you-might-be-making-when-using-kotlin-coroutines/

То есть всегда-всегда нужно пробрасывать дальше CancellationException, даже если отмена корутины не нужна?

Все-таки не совсем понимаю, почему в целом будет плохо отловить CancellationException, а не пробросить его. Ну то есть у нас есть, допустим, случай, что нам не нужно отменять Крутину, почему просто не словить CancellationException. Какие могут быть последствия? То, что, например, ViewModel отменят скоп по уничтожению, он отменяет всех своих детей, но эта Крутина продолжает работать, так как перехватила CancellationException?

SergeyKozhukhov · 2023-07-16T18:51:53Z

app/src/main/java/otus/homework/coroutines/presentation/mvp/CatsView.kt

+
+    override fun warn(message: String) {
+        Toast.makeText(context, message, Toast.LENGTH_SHORT).show()
+    }


Может слишком очевидный вопрос, но это ок, дергать Toast на этом контексте? Не надо ли applicationContext сделать?

Тост можно показывать на любом контексте

Имел ввиду, что если правильно понимаю, внутри View будет context activity получаться через context, так как она в нем лежит. Но не будет ли утечек никаких, если этот контент использовать для Toast-а внутри View, а не в самой Activity. Предполагаю, что все нормально будет

SergeyKozhukhov · 2023-07-16T18:52:54Z

app/src/main/java/otus/homework/coroutines/presentation/mvvm/CatsView.kt

+            } else {
+                toast.show()
+                viewModel.onErrorShown()
+            }


Чтобы повторно не показывать Toast при пересоздании

SergeyKozhukhov · 2023-07-16T18:54:07Z

app/src/main/java/otus/homework/coroutines/presentation/mvvm/CatsViewModel.kt

+     * Принудительное выполнение загрузки - `true`. Ленивое выполнение загрузки - `false`
+     * (загрузка будет выполняться только в случае, если она еще не проводилась)
+     */
+    fun getRandomCat(force: Boolean) {


force чтобы при attach не дергать доп запросы

SergeyKozhukhov · 2023-07-16T18:55:27Z

app/src/main/java/otus/homework/coroutines/presentation/mvvm/CatsViewModel.kt

+                onError(e)
+                if (e is CancellationException) {
+                    throw e
+                }


Здесь бросаю, так как viewModelScope - supervisor. Но опять же, есть ли смысл?

Лучше просто сделай catch на один тип исключений

Сделал

SergeyKozhukhov · 2023-07-16T18:57:44Z

app/src/main/java/otus/homework/coroutines/presentation/mvvm/owners/CatsView.kt

+        lifecycleScope.launch {
+            repeatOnLifecycle(Lifecycle.State.CREATED) {
+                viewModel.uiState.collect { state -> update(state) }
+            }


Lifecycle.State.CREATED - так как здесь только он и есть.
Стоит ли дополнять другими состояния - вопрос.

SergeyKozhukhov · 2023-07-16T18:59:29Z

app/src/main/java/otus/homework/coroutines/presentation/mvvm/owners/CatsView.kt

+    override fun onDetachedFromWindow() {
+        lifecycleRegistry.handleLifecycleEvent(Lifecycle.Event.ON_DESTROY)
+        viewModelStore.clear()
+        super.onDetachedFromWindow()


Хотелось бы, чтобы ViewModel переживала пересоздание, например, activity. Но как это сделать??? Подскажите, пожалуйста

Так она же итак переживает, в этом ее особенность. Если ты ее правильно создаешь через фабрику то все ок

В этой CatsView реализую самостоятельно ViewModelStore, LifecycleRegistry и наследую View от ViewModelStoreOwner, LifecycleOwner. Через lifecycleRegistry вызываю Event-ы на ON_CREATE и ON_DESTROY.
То есть ViewModel закрепляется за самой View, а не за Activity или Fragment-ом. Получаю ViewModel через фабрику.
Но хотелось бы, чтобы при, например, повороте экрана запрос из ViewModel продолжил бы выполняться. В целом, чтобы ViewModel осталась жива. Но как это сделать - не придумал. ViewModelStore очищается при повороте экрана и ViewModel создается заново. Не знаю, насколько нормально придумывать что-то с передачей через Bundle. Какие-то практики может подскажите?)

SergeyKozhukhov · 2023-07-30T10:40:41Z

app/src/main/java/otus/homework/coroutines/data/FactService.kt

+
+    /** Получить случайных факт о кошке */
+    @GET("fact")
+    suspend fun getCatFact(): Fact


Какие ошибки может выбрасывать Retrofit здесь?

Зависит от статус кода который придет, ну точно может выбросить Таймаут, в целом все равно все статус коды которые не входят в 200-299 будут расценены колладаптером как исключение

Если нужно подробнее глянь реализацию колладаптеров

Спасибо! Пока кажется, что с ретрофитом мы не можем четко прописать бросаемые исключения, так как они внутри него самого прописаны и их могут менять

SergeyKozhukhov · 2023-07-30T10:41:28Z

app/src/main/java/otus/homework/coroutines/di/DiContainer.kt

+    /** Репозиторий информации о кошке */
+    val catRepository: CatRepository by lazy(LazyThreadSafetyMode.NONE) {
+        CatRepositoryImpl(provideFactService(), provideImagesService(), CatConverter())
+    }


Правильно понимаю, что здесь нормально ставить LazyThreadSafetyMode.NONE? Или все-таки могут быть проблемы с синхронизацией?

Все верно, так гораздо лучше

SergeyKozhukhov · 2023-07-30T10:50:26Z

app/src/main/java/otus/homework/coroutines/presentation/mvvm/CatsViewModel.kt

+            try {
+                val catInfo = repository.getCatInfo()
+                _uiState.update { CatUiState.Success(catInfo) }
+            } catch (e: Exception) {


Выполняю обновление через update, правильно понимаю, что это предпочтительный способ и его стоит везде использовать вместе
_uiState.value = ... ?

Честно первый раз слышу про апдейт. Я знаю про emit и value. Что делает update?

Вот здесь хорошо описано
https://scribe.rip/geekculture/atomic-updates-with-mutablestateflow-dc0331724405
В примерах у гугла, кажется, его видел еще. Но не нашел что-то сейчас.

update синхронизировано обновляет значение.

В целом кажется, что здесь может быть излишне, здесь не использую IO и Default

antonkazakov

Привет. Ответил на вопросы, замечаний вроде нет

antonkazakov · 2023-07-30T13:57:38Z

app/src/main/java/otus/homework/coroutines/data/FactService.kt

+
+    /** Получить случайных факт о кошке */
+    @GET("fact")
+    suspend fun getCatFact(): Fact


Зависит от статус кода который придет, ну точно может выбросить Таймаут, в целом все равно все статус коды которые не входят в 200-299 будут расценены колладаптером как исключение

antonkazakov · 2023-07-30T13:57:52Z

app/src/main/java/otus/homework/coroutines/data/FactService.kt

+
+    /** Получить случайных факт о кошке */
+    @GET("fact")
+    suspend fun getCatFact(): Fact


Если нужно подробнее глянь реализацию колладаптеров

antonkazakov · 2023-07-30T13:59:15Z

app/src/main/java/otus/homework/coroutines/di/DiContainer.kt

+    /** Репозиторий информации о кошке */
+    val catRepository: CatRepository by lazy(LazyThreadSafetyMode.NONE) {
+        CatRepositoryImpl(provideFactService(), provideImagesService(), CatConverter())
+    }


Все верно, так гораздо лучше

antonkazakov · 2023-07-30T14:00:30Z

app/src/main/java/otus/homework/coroutines/models/presentation/CatUiState.kt

+     * @param isShown признак отображения причины отсутствия данных
+     */
+    data class Error(val message: String, val isShown: Boolean = false) : CatUiState()
+}


Идея понятная, но если ты хочешь ее решить совсем правильно то тебе нужно 2 лайвдаты/стейтфлоу. Один который с синглевентами/реплеями а другой без

antonkazakov · 2023-07-30T14:02:07Z

app/src/main/java/otus/homework/coroutines/presentation/mvp/CatsPresenter.kt

+                } else {
+                    CrashMonitor.trackWarning(e)
+                    _catsView?.warn(message = e.messageOrDefault())
+                }


Если хочешь чтобы не отменялся то целевой механизм это супервайзор

antonkazakov · 2023-07-30T14:02:43Z

app/src/main/java/otus/homework/coroutines/presentation/mvp/CatsPresenter.kt

+                } else {
+                    CrashMonitor.trackWarning(e)
+                    _catsView?.warn(message = e.messageOrDefault())
+                }


Эксепшен лучше пробросить, либо написать catch на нужные типы исключений. Ну либо взять ексепшенхендлер

antonkazakov · 2023-07-30T14:03:07Z

app/src/main/java/otus/homework/coroutines/presentation/mvp/CatsView.kt

+
+    override fun warn(message: String) {
+        Toast.makeText(context, message, Toast.LENGTH_SHORT).show()
+    }


Тост можно показывать на любом контексте

antonkazakov · 2023-07-30T14:04:09Z

app/src/main/java/otus/homework/coroutines/presentation/mvvm/CatsViewModel.kt

+            try {
+                val catInfo = repository.getCatInfo()
+                _uiState.update { CatUiState.Success(catInfo) }
+            } catch (e: Exception) {


Честно первый раз слышу про апдейт. Я знаю про emit и value. Что делает update?

antonkazakov · 2023-07-30T14:04:31Z

app/src/main/java/otus/homework/coroutines/presentation/mvvm/CatsViewModel.kt

+                onError(e)
+                if (e is CancellationException) {
+                    throw e
+                }


Лучше просто сделай catch на один тип исключений

antonkazakov · 2023-07-30T14:05:09Z

app/src/main/java/otus/homework/coroutines/presentation/mvvm/owners/CatsView.kt

+    override fun onDetachedFromWindow() {
+        lifecycleRegistry.handleLifecycleEvent(Lifecycle.Event.ON_DESTROY)
+        viewModelStore.clear()
+        super.onDetachedFromWindow()


Так она же итак переживает, в этом ее особенность. Если ты ее правильно создаешь через фабрику то все ок

SergeyKozhukhov · 2023-08-04T07:54:48Z

app/src/main/java/otus/homework/coroutines/presentation/mvvm/owners/CatsViewModel.kt

+
+    /** Событие ошибки */
+    val errorEvents get() = _errorEvent.receiveAsFlow()
+    private val _errorEvent = Channel<String>(CONFLATED)


Сделал CONFLATED, но может есть лучше практики

SergeyKozhukhov · 2023-08-04T07:56:24Z

app/src/main/java/otus/homework/coroutines/presentation/mvvm/owners/CatsViewModel.kt

+        CrashMonitor.trackWarning(e)
+        viewModelScope.launch(dispatcher.mainImmediate) {
+            _errorEvent.send(
+                stringProvider.getString(R.string.unexpected_request_error, e.messageOrDefault())


Здесь делаю launch на main.immeduate по вот этому описанию
Kotlin/kotlinx.coroutines#2886 (comment)

SergeyKozhukhov · 2023-08-04T07:57:23Z

app/src/main/java/otus/homework/coroutines/presentation/mvvm/owners/CatsView.kt

+                }
+                launch(Dispatchers.Main.immediate) {
+                    viewModel.errorEvents.collect { message -> onError(message) }
+                }


Здесь также для Channel-а использую Dispatchers.Main.immediate
по той статье

SergeyKozhukhov · 2023-08-04T07:58:22Z

README.md

+1. ```/mvp``` - реализация *custom view* на основе паттеран `MVP`
+2. ```/mvvm/parent``` - реализация *custom view* на основе паттерна ```MVVM``` и `findViewTreeViewModelStoreOwner`
+3. ```/mvvm/owners``` - реализация *custom view* на основе паттерна ```MVVM``` и *custom* ```ViewModelStoreOwner, LifecycleOwner```
+4. ```/mvi``` - реализация *custom view* на основе паттерна ```MVI``` и библиотеки ```MVICore```


Добавил реализацию на MVI, но скорее в качестве эксперимента, там coroutine толком нет

SergeyKozhukhov added 5 commits July 16, 2023 20:29

First step: from callbacks to suspend functions and coroutines

ffa0814

Second step: random image request

21557eb

Third step: use ViewModel

56e9c98

Third step: custom ViewModelStoreOwner and LifecycleOwner

4975aa0

- add custom view with custom ViewModelStoreOwner and LifecycleOwner - refactoring

Other: add KDoc and update README

1085f3d

SergeyKozhukhov commented Jul 16, 2023

View reviewed changes

SergeyKozhukhov commented Jul 30, 2023

View reviewed changes

antonkazakov requested changes Jul 30, 2023

View reviewed changes

SergeyKozhukhov added 2 commits August 4, 2023 10:11

Improvement: fixes, changing packages, KDoc

5852c57

Experiment: use MVICore

9b2f20c

SergeyKozhukhov commented Aug 4, 2023

View reviewed changes

Задача по Coroutines #160

Are you sure you want to change the base?

Задача по Coroutines #160

Conversation

SergeyKozhukhov commented Jul 16, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SergeyKozhukhov Aug 4, 2023 • edited Loading

Choose a reason for hiding this comment

SergeyKozhukhov Jul 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SergeyKozhukhov Jul 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SergeyKozhukhov Aug 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SergeyKozhukhov Jul 16, 2023 • edited Loading

Choose a reason for hiding this comment

SergeyKozhukhov Jul 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antonkazakov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SergeyKozhukhov Aug 4, 2023 •

edited

Loading

SergeyKozhukhov Jul 16, 2023 •

edited

Loading

SergeyKozhukhov Jul 16, 2023 •

edited

Loading

SergeyKozhukhov Aug 4, 2023 •

edited

Loading

SergeyKozhukhov Jul 16, 2023 •

edited

Loading

SergeyKozhukhov Jul 16, 2023 •

edited

Loading