From 581d1cf05afb54a23bac26ec705d6ddc7b6498eb Mon Sep 17 00:00:00 2001 From: Akashkamble Date: Wed, 9 Sep 2020 12:26:03 +0530 Subject: [PATCH] Fix ArrayIndex out of bound exception when navigating between two categories having different number of articles. --- app/build.gradle | 4 +- .../akash/newzz_compose/ui/NewzzActivity.kt | 2 - .../newzz_compose/ui/newzzappui/NewzzAppUI.kt | 53 ++++++++++++++--- .../newzz_compose/viewmodel/NewzzViewModel.kt | 58 +++++++++++++------ 4 files changed, 87 insertions(+), 30 deletions(-) diff --git a/app/build.gradle b/app/build.gradle index 2967507..aea72db 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -12,8 +12,8 @@ android { applicationId "com.akash.newzzcompose" minSdkVersion 21 targetSdkVersion 29 - versionCode 3 - versionName "0.0.3" + versionCode 4 + versionName "0.0.4" testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner" } diff --git a/app/src/main/java/com/akash/newzz_compose/ui/NewzzActivity.kt b/app/src/main/java/com/akash/newzz_compose/ui/NewzzActivity.kt index 072f087..0077df7 100644 --- a/app/src/main/java/com/akash/newzz_compose/ui/NewzzActivity.kt +++ b/app/src/main/java/com/akash/newzz_compose/ui/NewzzActivity.kt @@ -1,7 +1,6 @@ package com.akash.newzz_compose.ui import android.os.Bundle -import android.util.Log import androidx.activity.viewModels import androidx.appcompat.app.AppCompatActivity import androidx.compose.runtime.livedata.observeAsState @@ -23,7 +22,6 @@ class NewzzActivity : AppCompatActivity() { super.onCreate(savedInstanceState) setContent { val darkTheme = viewModel.isDarkTheme.observeAsState(false) - Log.d("THEME", "darkTheme: ${darkTheme.value}") NewzzTheme(darkTheme = darkTheme.value) { NewzzAppUI(viewModel = viewModel) } diff --git a/app/src/main/java/com/akash/newzz_compose/ui/newzzappui/NewzzAppUI.kt b/app/src/main/java/com/akash/newzz_compose/ui/newzzappui/NewzzAppUI.kt index 9370667..392685f 100644 --- a/app/src/main/java/com/akash/newzz_compose/ui/newzzappui/NewzzAppUI.kt +++ b/app/src/main/java/com/akash/newzz_compose/ui/newzzappui/NewzzAppUI.kt @@ -20,7 +20,15 @@ import com.akash.newzz_compose.viewmodel.NewzzViewModel fun NewzzAppUI(viewModel: NewzzViewModel) { val categoryList = viewModel.categoryList.observeAsState().value!! val activeCategory = viewModel.activeCategory.observeAsState().value!! - val activeCategoryUiState = viewModel.activeCategoryUiState.observeAsState().value!! + val firstCategoryUiState = viewModel.firstCategoryUiState.observeAsState().value!! + val secondCategoryUiState = viewModel.secondCategoryUiState.observeAsState().value!! + val thirdCategoryUiState = viewModel.thirdCategoryUiState.observeAsState().value!! + val listOfUiState = listOf( + firstCategoryUiState, + secondCategoryUiState, + thirdCategoryUiState + ) + val activeIndex = categoryList.indexOf(activeCategory) Scaffold( bodyContent = { BodyContent( @@ -28,7 +36,8 @@ fun NewzzAppUI(viewModel: NewzzViewModel) { onThemeSwitch = { viewModel.performAction(NewzzViewModel.Action.SwitchTheme) }, - activeCategoryUiState = activeCategoryUiState, + activeCategoryUiStateList = listOfUiState, + activeIndex = activeIndex, retryFetchingArticles = { category -> viewModel.performAction(NewzzViewModel.Action.FetchArticles(category)) } @@ -49,8 +58,9 @@ fun NewzzAppUI(viewModel: NewzzViewModel) { @Composable fun BodyContent( activeCategory: Category, - activeCategoryUiState: ArticleListUiState, + activeCategoryUiStateList: List, onThemeSwitch: () -> Unit, + activeIndex: Int, retryFetchingArticles: (Category) -> Unit ) { val stringRes = getTitleResource(activeCategory) @@ -62,12 +72,37 @@ fun BodyContent( TopAppBar(stringRes, onThemeSwitch = { onThemeSwitch() }) - NewzzListContainer( - uiState = activeCategoryUiState, - retry = { - retryFetchingArticles(activeCategory) - } - ) + /* This when statement does not make any sense. Need to figure out better solution. + the idea was to change uiState based on activeCategory, but when 2 category has different number of articles + and you switch between pages LazyColumnFor remembers scroll position of previous state which leads to ArrayIndexOutOfBound Exception. + */ + when (activeIndex) { + 0 -> { + NewzzListContainer( + uiState = activeCategoryUiStateList[activeIndex], + retry = { + retryFetchingArticles(activeCategory) + } + ) + } + 1 -> { + NewzzListContainer( + uiState = activeCategoryUiStateList[activeIndex], + retry = { + retryFetchingArticles(activeCategory) + } + ) + } + 2 -> { + NewzzListContainer( + uiState = activeCategoryUiStateList[activeIndex], + retry = { + retryFetchingArticles(activeCategory) + } + ) + } + } + } } } diff --git a/app/src/main/java/com/akash/newzz_compose/viewmodel/NewzzViewModel.kt b/app/src/main/java/com/akash/newzz_compose/viewmodel/NewzzViewModel.kt index 83f3bd8..20e5f22 100644 --- a/app/src/main/java/com/akash/newzz_compose/viewmodel/NewzzViewModel.kt +++ b/app/src/main/java/com/akash/newzz_compose/viewmodel/NewzzViewModel.kt @@ -35,14 +35,26 @@ class NewzzViewModel @ViewModelInject constructor(private val repo: NewsReposito } val activeCategory: LiveData = _activeCategory - private val _activeCategoryUiState = MutableLiveData().apply { + /*private val _activeCategoryUiState = MutableLiveData().apply { value = ArticleListUiState() } - val activeCategoryUiState: LiveData = _activeCategoryUiState + private val activeCategoryUiState: LiveData = _activeCategoryUiState*/ private var firstPageUiState = ArticleListUiState() + private val _firstCategoryUiState = MutableLiveData().apply { + value = ArticleListUiState() + } + val firstCategoryUiState: LiveData = _firstCategoryUiState private var secondPageUiState = ArticleListUiState() + private val _secondCategoryUiState = MutableLiveData().apply { + value = ArticleListUiState() + } + val secondCategoryUiState: LiveData = _secondCategoryUiState private var thirdPageUiState = ArticleListUiState() + private val _thirdCategoryUiState = MutableLiveData().apply { + value = ArticleListUiState() + } + val thirdCategoryUiState: LiveData = _thirdCategoryUiState init { getArticlesByCategory(categoryList.value!![0]) @@ -74,12 +86,24 @@ class NewzzViewModel @ViewModelInject constructor(private val repo: NewsReposito is Action.ChangePageTo -> { _activeCategory.value = action.category when (categoryList.value!!.indexOf(action.category)) { - 0 -> _activeCategoryUiState.value = firstPageUiState - 1 -> _activeCategoryUiState.value = secondPageUiState - 2 -> _activeCategoryUiState.value = thirdPageUiState - } - if (activeCategoryUiState.value!!.list == null || activeCategoryUiState.value!!.list!!.isEmpty()) { - getArticlesByCategory(action.category) + 0 -> { + _firstCategoryUiState.value = firstPageUiState + if (_firstCategoryUiState.value!!.list == null || _firstCategoryUiState.value!!.list!!.isEmpty()) { + getArticlesByCategory(action.category) + } + } + 1 -> { + _secondCategoryUiState.value = secondPageUiState + if (_secondCategoryUiState.value!!.list == null || _secondCategoryUiState.value!!.list!!.isEmpty()) { + getArticlesByCategory(action.category) + } + } + 2 -> { + _thirdCategoryUiState.value = thirdPageUiState + if (_thirdCategoryUiState.value!!.list == null || _thirdCategoryUiState.value!!.list!!.isEmpty()) { + getArticlesByCategory(action.category) + } + } } } is Action.FetchArticles -> { @@ -99,7 +123,7 @@ class NewzzViewModel @ViewModelInject constructor(private val repo: NewsReposito list = null, error = error ) - _activeCategoryUiState.value = firstPageUiState + _firstCategoryUiState.value = firstPageUiState } 1 -> { secondPageUiState = secondPageUiState.copy( @@ -107,7 +131,7 @@ class NewzzViewModel @ViewModelInject constructor(private val repo: NewsReposito list = null, error = error ) - _activeCategoryUiState.value = secondPageUiState + _secondCategoryUiState.value = secondPageUiState } 2 -> { thirdPageUiState = thirdPageUiState.copy( @@ -115,7 +139,7 @@ class NewzzViewModel @ViewModelInject constructor(private val repo: NewsReposito list = null, error = error ) - _activeCategoryUiState.value = thirdPageUiState + _thirdCategoryUiState.value = thirdPageUiState } } } @@ -128,7 +152,7 @@ class NewzzViewModel @ViewModelInject constructor(private val repo: NewsReposito list = data.articles, error = null ) - _activeCategoryUiState.value = firstPageUiState + _firstCategoryUiState.value = firstPageUiState } 1 -> { secondPageUiState = secondPageUiState.copy( @@ -136,7 +160,7 @@ class NewzzViewModel @ViewModelInject constructor(private val repo: NewsReposito list = data.articles, error = null ) - _activeCategoryUiState.value = secondPageUiState + _secondCategoryUiState.value = secondPageUiState } 2 -> { thirdPageUiState = thirdPageUiState.copy( @@ -144,7 +168,7 @@ class NewzzViewModel @ViewModelInject constructor(private val repo: NewsReposito list = data.articles, error = null ) - _activeCategoryUiState.value = thirdPageUiState + _thirdCategoryUiState.value = thirdPageUiState } } } @@ -152,13 +176,13 @@ class NewzzViewModel @ViewModelInject constructor(private val repo: NewsReposito private fun setLoadingState(category: Category) { when (categoryList.value!!.indexOf(category)) { 0 -> { - _activeCategoryUiState.value = firstPageUiState.copy(isLoading = true) + _firstCategoryUiState.value = firstPageUiState.copy(isLoading = true) } 1 -> { - _activeCategoryUiState.value = secondPageUiState.copy(isLoading = true) + _secondCategoryUiState.value = secondPageUiState.copy(isLoading = true) } 2 -> { - _activeCategoryUiState.value = thirdPageUiState.copy(isLoading = true) + _thirdCategoryUiState.value = thirdPageUiState.copy(isLoading = true) } } }