Skip to content

Commit

Permalink
Fix ArrayIndex out of bound exception when navigating between two cat…
Browse files Browse the repository at this point in the history
…egories having different number of articles.
  • Loading branch information
Akashkamble committed Sep 9, 2020
1 parent bdf4228 commit 581d1cf
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 30 deletions.
4 changes: 2 additions & 2 deletions app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}

Expand Down
2 changes: 0 additions & 2 deletions app/src/main/java/com/akash/newzz_compose/ui/NewzzActivity.kt
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,24 @@ 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(
activeCategory = activeCategory,
onThemeSwitch = {
viewModel.performAction(NewzzViewModel.Action.SwitchTheme)
},
activeCategoryUiState = activeCategoryUiState,
activeCategoryUiStateList = listOfUiState,
activeIndex = activeIndex,
retryFetchingArticles = { category ->
viewModel.performAction(NewzzViewModel.Action.FetchArticles(category))
}
Expand All @@ -49,8 +58,9 @@ fun NewzzAppUI(viewModel: NewzzViewModel) {
@Composable
fun BodyContent(
activeCategory: Category,
activeCategoryUiState: ArticleListUiState,
activeCategoryUiStateList: List<ArticleListUiState>,
onThemeSwitch: () -> Unit,
activeIndex: Int,
retryFetchingArticles: (Category) -> Unit
) {
val stringRes = getTitleResource(activeCategory)
Expand All @@ -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)
}
)
}
}

}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,26 @@ class NewzzViewModel @ViewModelInject constructor(private val repo: NewsReposito
}
val activeCategory: LiveData<Category> = _activeCategory

private val _activeCategoryUiState = MutableLiveData<ArticleListUiState>().apply {
/*private val _activeCategoryUiState = MutableLiveData<ArticleListUiState>().apply {
value = ArticleListUiState()
}
val activeCategoryUiState: LiveData<ArticleListUiState> = _activeCategoryUiState
private val activeCategoryUiState: LiveData<ArticleListUiState> = _activeCategoryUiState*/

private var firstPageUiState = ArticleListUiState()
private val _firstCategoryUiState = MutableLiveData<ArticleListUiState>().apply {
value = ArticleListUiState()
}
val firstCategoryUiState: LiveData<ArticleListUiState> = _firstCategoryUiState
private var secondPageUiState = ArticleListUiState()
private val _secondCategoryUiState = MutableLiveData<ArticleListUiState>().apply {
value = ArticleListUiState()
}
val secondCategoryUiState: LiveData<ArticleListUiState> = _secondCategoryUiState
private var thirdPageUiState = ArticleListUiState()
private val _thirdCategoryUiState = MutableLiveData<ArticleListUiState>().apply {
value = ArticleListUiState()
}
val thirdCategoryUiState: LiveData<ArticleListUiState> = _thirdCategoryUiState

init {
getArticlesByCategory(categoryList.value!![0])
Expand Down Expand Up @@ -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 -> {
Expand All @@ -99,23 +123,23 @@ class NewzzViewModel @ViewModelInject constructor(private val repo: NewsReposito
list = null,
error = error
)
_activeCategoryUiState.value = firstPageUiState
_firstCategoryUiState.value = firstPageUiState
}
1 -> {
secondPageUiState = secondPageUiState.copy(
isLoading = false,
list = null,
error = error
)
_activeCategoryUiState.value = secondPageUiState
_secondCategoryUiState.value = secondPageUiState
}
2 -> {
thirdPageUiState = thirdPageUiState.copy(
isLoading = false,
list = null,
error = error
)
_activeCategoryUiState.value = thirdPageUiState
_thirdCategoryUiState.value = thirdPageUiState
}
}
}
Expand All @@ -128,37 +152,37 @@ class NewzzViewModel @ViewModelInject constructor(private val repo: NewsReposito
list = data.articles,
error = null
)
_activeCategoryUiState.value = firstPageUiState
_firstCategoryUiState.value = firstPageUiState
}
1 -> {
secondPageUiState = secondPageUiState.copy(
isLoading = false,
list = data.articles,
error = null
)
_activeCategoryUiState.value = secondPageUiState
_secondCategoryUiState.value = secondPageUiState
}
2 -> {
thirdPageUiState = thirdPageUiState.copy(
isLoading = false,
list = data.articles,
error = null
)
_activeCategoryUiState.value = thirdPageUiState
_thirdCategoryUiState.value = thirdPageUiState
}
}
}

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)
}
}
}
Expand Down

0 comments on commit 581d1cf

Please sign in to comment.