Replace Either with Kotlin Result and optimize ListUtils (#4443)

Using `Either<Throwable, T>` is basically the same as `Result<T>` with a
less friendly API. Futhermore, `Either` is currently only used in a
single component.

- Replace `Either` with `Result` in `AccountsInListFragment` and
`AccountsInListViewModel`.
- Add a method to convert a `NetworkResult` to a `Result` in
`AccountsInListViewModel`. Alternatively, `NetworkResult` could be used
everywhere in the code but other classes are already using `Result`.
- Replace `updateState()` method with `MutableStateFlow.update()` in
`AccountsInListViewModel`.
- Store the current search query in a `MutableStateFlow` collected by a
coroutine. This allows automatically cancelling the previous search when
a new query arrives, instead of launching a new coroutine for each query
which may conflict with the previous ones.
- Optimize `ListUtils`.
This commit is contained in:
Christophe Beyls 2024-05-31 12:58:24 +02:00 committed by GitHub
commit 2cbd629150
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 72 additions and 114 deletions

View file

@ -34,7 +34,6 @@ import com.keylesspalace.tusky.databinding.ItemFollowRequestBinding
import com.keylesspalace.tusky.entity.TimelineAccount
import com.keylesspalace.tusky.settings.PrefKeys
import com.keylesspalace.tusky.util.BindingHolder
import com.keylesspalace.tusky.util.Either
import com.keylesspalace.tusky.util.emojify
import com.keylesspalace.tusky.util.hide
import com.keylesspalace.tusky.util.loadAvatar
@ -103,12 +102,12 @@ class AccountsInListFragment : DialogFragment() {
viewLifecycleOwner.lifecycleScope.launch {
viewModel.state.collect { state ->
adapter.submitList(state.accounts.asRightOrNull() ?: listOf())
adapter.submitList(state.accounts.getOrDefault(emptyList()))
when (state.accounts) {
is Either.Right -> binding.messageView.hide()
is Either.Left -> handleError(state.accounts.value)
}
state.accounts.fold(
onSuccess = { binding.messageView.hide() },
onFailure = { handleError(it) }
)
setupSearchView(state)
}
@ -137,7 +136,7 @@ class AccountsInListFragment : DialogFragment() {
binding.accountsSearchRecycler.hide()
binding.accountsRecycler.show()
} else {
val listAccounts = state.accounts.asRightOrNull() ?: listOf()
val listAccounts = state.accounts.getOrDefault(emptyList())
val newList = state.searchResult.map { acc ->
acc to listAccounts.contains(acc)
}

View file

@ -21,7 +21,7 @@ import com.keylesspalace.tusky.databinding.ItemFooterBinding
import com.keylesspalace.tusky.entity.TimelineAccount
import com.keylesspalace.tusky.interfaces.AccountActionListener
import com.keylesspalace.tusky.util.BindingHolder
import com.keylesspalace.tusky.util.removeDuplicates
import com.keylesspalace.tusky.util.removeDuplicatesTo
/** Generic adapter with bottom loading indicator. */
abstract class AccountAdapter<AVH : RecyclerView.ViewHolder> internal constructor(
@ -74,7 +74,7 @@ abstract class AccountAdapter<AVH : RecyclerView.ViewHolder> internal constructo
}
fun update(newAccounts: List<TimelineAccount>) {
accountList = removeDuplicates(newAccounts)
accountList = newAccounts.removeDuplicatesTo(ArrayList())
notifyDataSetChanged()
}

View file

@ -1,49 +0,0 @@
/* Copyright 2017 Andrew Dawson
*
* This file is a part of Tusky.
*
* This program is free software; you can redistribute it and/or modify it under the terms of the
* GNU General Public License as published by the Free Software Foundation; either version 3 of the
* License, or (at your option) any later version.
*
* Tusky is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even
* the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General
* Public License for more details.
*
* You should have received a copy of the GNU General Public License along with Tusky; if not,
* see <http://www.gnu.org/licenses>. */
package com.keylesspalace.tusky.util
/**
* Created by charlag on 05/11/17.
*
* Class to represent sum type/tagged union/variant/ADT e.t.c.
* It is either Left or Right.
*/
sealed interface Either<out L, out R> {
data class Left<out L, out R>(val value: L) : Either<L, R>
data class Right<out L, out R>(val value: R) : Either<L, R>
fun isRight(): Boolean = this is Right
fun isLeft(): Boolean = this is Left
fun asLeftOrNull(): L? = (this as? Left<L, R>)?.value
fun asRightOrNull(): R? = (this as? Right<L, R>)?.value
fun asLeft(): L = (this as Left<L, R>).value
fun asRight(): R = (this as Right<L, R>).value
companion object {
inline fun <L, R, N> Either<L, R>.map(mapper: (R) -> N): Either<L, N> {
return if (this.isLeft()) {
Left(this.asLeft())
} else {
Right(mapper(this.asRight()))
}
}
}
}

View file

@ -17,31 +17,36 @@
package com.keylesspalace.tusky.util
import java.util.ArrayList
import java.util.LinkedHashSet
/**
* Copies elements to destination, removing duplicates and preserving original order.
*/
fun <T, C : MutableCollection<in T>> Iterable<T>.removeDuplicatesTo(destination: C): C {
return filterTo(destination, HashSet<T>()::add)
}
/**
* @return a new ArrayList containing the elements without duplicates in the same order
* Copies elements to a new list, removing duplicates and preserving original order.
*/
fun <T> removeDuplicates(list: List<T>): ArrayList<T> {
val set = LinkedHashSet(list)
return ArrayList(set)
fun <T> Iterable<T>.removeDuplicates(): List<T> {
return removeDuplicatesTo(ArrayList())
}
inline fun <T> List<T>.withoutFirstWhich(predicate: (T) -> Boolean): List<T> {
val newList = toMutableList()
val index = newList.indexOfFirst(predicate)
if (index != -1) {
newList.removeAt(index)
val index = indexOfFirst(predicate)
if (index == -1) {
return this
}
val newList = toMutableList()
newList.removeAt(index)
return newList
}
inline fun <T> List<T>.replacedFirstWhich(replacement: T, predicate: (T) -> Boolean): List<T> {
val newList = toMutableList()
val index = newList.indexOfFirst(predicate)
if (index != -1) {
newList[index] = replacement
val index = indexOfFirst(predicate)
if (index == -1) {
return this
}
val newList = toMutableList()
newList[index] = replacement
return newList
}

View file

@ -19,22 +19,22 @@ package com.keylesspalace.tusky.viewmodel
import android.util.Log
import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import at.connyduck.calladapter.networkresult.NetworkResult
import at.connyduck.calladapter.networkresult.fold
import at.connyduck.calladapter.networkresult.getOrDefault
import com.keylesspalace.tusky.entity.TimelineAccount
import com.keylesspalace.tusky.network.MastodonApi
import com.keylesspalace.tusky.util.Either
import com.keylesspalace.tusky.util.Either.Companion.map
import com.keylesspalace.tusky.util.Either.Left
import com.keylesspalace.tusky.util.Either.Right
import com.keylesspalace.tusky.util.withoutFirstWhich
import dagger.hilt.android.lifecycle.HiltViewModel
import javax.inject.Inject
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.collectLatest
import kotlinx.coroutines.flow.update
import kotlinx.coroutines.launch
data class State(
val accounts: Either<Throwable, List<TimelineAccount>>,
val accounts: Result<List<TimelineAccount>>,
val searchResult: List<TimelineAccount>?
)
@ -42,20 +42,19 @@ data class State(
class AccountsInListViewModel @Inject constructor(private val api: MastodonApi) : ViewModel() {
val state: Flow<State> get() = _state
private val _state = MutableStateFlow(State(Right(listOf()), null))
private val _state = MutableStateFlow(
State(
accounts = Result.success(emptyList()),
searchResult = null
)
)
fun load(listId: String) {
val state = _state.value
if (state.accounts.isLeft() || state.accounts.asRight().isEmpty()) {
if (state.accounts.isFailure || state.accounts.getOrThrow().isEmpty()) {
viewModelScope.launch {
api.getAccountsInList(listId, 0).fold(
{ accounts ->
updateState { copy(accounts = Right(accounts)) }
},
{ e ->
updateState { copy(accounts = Left(e)) }
}
)
val accounts = api.getAccountsInList(listId, 0)
_state.update { it.copy(accounts = accounts.toResult()) }
}
}
}
@ -64,14 +63,14 @@ class AccountsInListViewModel @Inject constructor(private val api: MastodonApi)
viewModelScope.launch {
api.addAccountToList(listId, listOf(account.id))
.fold(
{
updateState {
copy(accounts = accounts.map { it + account })
onSuccess = {
_state.update { state ->
state.copy(accounts = state.accounts.map { it + account })
}
},
{
onFailure = {
Log.i(
javaClass.simpleName,
AccountsInListViewModel::class.java.simpleName,
"Failed to add account to list: ${account.username}"
)
}
@ -83,18 +82,18 @@ class AccountsInListViewModel @Inject constructor(private val api: MastodonApi)
viewModelScope.launch {
api.deleteAccountFromList(listId, listOf(accountId))
.fold(
{
updateState {
copy(
accounts = accounts.map { accounts ->
onSuccess = {
_state.update { state ->
state.copy(
accounts = state.accounts.map { accounts ->
accounts.withoutFirstWhich { it.id == accountId }
}
)
}
},
{
onFailure = {
Log.i(
javaClass.simpleName,
AccountsInListViewModel::class.java.simpleName,
"Failed to remove account from list: $accountId"
)
}
@ -102,25 +101,29 @@ class AccountsInListViewModel @Inject constructor(private val api: MastodonApi)
}
}
private val currentQuery = MutableStateFlow("")
fun search(query: String) {
when {
query.isEmpty() -> updateState { copy(searchResult = null) }
query.isBlank() -> updateState { copy(searchResult = listOf()) }
else -> viewModelScope.launch {
api.searchAccounts(query, null, 10, true)
.fold(
{ result ->
updateState { copy(searchResult = result) }
},
{
updateState { copy(searchResult = listOf()) }
}
)
currentQuery.value = query
}
init {
viewModelScope.launch {
// Use collectLatest to automatically cancel the previous search
currentQuery.collectLatest { query ->
val searchResult = when {
query.isEmpty() -> null
query.isBlank() -> emptyList()
else -> api.searchAccounts(query, null, 10, true)
.getOrDefault(emptyList())
}
_state.update { it.copy(searchResult = searchResult) }
}
}
}
private inline fun updateState(fn: State.() -> State) {
_state.value = fn(_state.value)
}
private fun <T> NetworkResult<T>.toResult(): Result<T> = fold(
onSuccess = { Result.success(it) },
onFailure = { Result.failure(it) }
)
}