Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace Either with Kotlin Result and optimize ListUtils #4443

Merged
merged 3 commits into from
May 31, 2024

Conversation

cbeyls
Copy link
Contributor

@cbeyls cbeyls commented May 12, 2024

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.

@connyduck
Copy link
Collaborator

The whole account list component is due for a rewrite (move to new package structure, ViewModel & AndroidX paging), I wanted to tackle that soon, but if you want you can do it as well

@cbeyls
Copy link
Contributor Author

cbeyls commented May 30, 2024

The whole account list component is due for a rewrite (move to new package structure, ViewModel & AndroidX paging), I wanted to tackle that soon, but if you want you can do it as well

I can help. This branch can be used as a starting point too.

Copy link
Collaborator

@connyduck connyduck left a comment

Choose a reason for hiding this comment

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

Yes, let's merge this

@connyduck connyduck merged commit 2cbd629 into tuskyapp:develop May 31, 2024
3 checks passed
@cbeyls cbeyls deleted the refactor/either_result branch June 1, 2024 14:50
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