Skip to content

Flow Homework#237

Open
prometey1936-dev wants to merge 4 commits intoOtus-Android:masterfrom
prometey1936-dev:master
Open

Flow Homework#237
prometey1936-dev wants to merge 4 commits intoOtus-Android:masterfrom
prometey1936-dev:master

Conversation

@prometey1936-dev
Copy link

No description provided.

_catsStateFlow.value = Result.Success(fact)
}
} catch (e: Exception) {
_catsStateFlow.value = Result.Error(e)

Choose a reason for hiding this comment

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

нужно добавить проброс CancellationException далее, чтобы не ломался механизм отмены корутин
if (e is CancellationException) throw e

Copy link
Author

Choose a reason for hiding this comment

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

Исправил

emit(latestNews)
delay(refreshIntervalMs)
}
}.catch { throwable ->

Choose a reason for hiding this comment

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

тут catch ничего не делает, лучше сделать возврат результата, при успехе
emit(Result.Success(catsService.getCatFact()))
а в catch

if (e is CancellationException) throw e
  emit(Result.Error(e))

Copy link
Author

Choose a reason for hiding this comment

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

Исправил

}
} catch (e: CancellationException) {
throw e
}

Choose a reason for hiding this comment

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

тут catch снова ничего не делает, он не ловит ошибки из сети, только захватывает и пробрасывает дальше исключения, нужно после этого блока тогда добавить еще один, плюс лучше из репозитория отдавать сразу Result:

        catch (e: Exception) {
            emit(Result.Error(e.message.orEmpty()))
        }

в выше эмитить
emit(Result.Success(latestNews))

во вью модель будет приходить сразу Result, где мы его уже дальше обрабатываем

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants