refactor: create PendingDepositRefreshHelper to consolidate pending d… #5750
Conversation
…eposit refresh logic
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the pending deposit refresh logic by extracting it into a new PendingDepositRefreshHelper utility class to eliminate code duplication across multiple wallet fragments.
- Consolidates duplicate pending deposit refresh logic from multiple fragments into a single helper class
- Implements lifecycle-aware job management for pending deposit refresh operations
- Removes the unused
clearPendingDepositsByAssetIdmethod fromWalletViewModel
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| WalletViewModel.kt | Removes unused clearPendingDepositsByAssetId method |
| TransactionsFragment.kt | Replaces custom refresh logic with PendingDepositRefreshHelper |
| PrivacyWalletFragment.kt | Replaces custom refresh logic with PendingDepositRefreshHelper |
| AllTransactionsFragment.kt | Replaces custom refresh logic with PendingDepositRefreshHelper |
| PendingDepositRefreshHelper.kt | Adds new helper class with consolidated pending deposit refresh logic |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| }, | ||
| ) | ||
|
|
||
| delay(10_000) |
There was a problem hiding this comment.
The infinite loop with fixed 10-second delay could lead to inefficient network requests and resource usage. Consider implementing exponential backoff on failures or a configurable refresh interval.
| delay(10_000) | |
| success = true | |
| }, | |
| ) | |
| // Exponential backoff logic | |
| if (success) { | |
| currentDelay = baseDelay | |
| } else { | |
| currentDelay = (currentDelay * 2).coerceAtMost(maxDelay) | |
| } | |
| delay(currentDelay) |
| delay(10_000) | ||
| } | ||
| } catch (e: Exception) { | ||
| Timber.e(e, "Error refreshing pending deposits") |
There was a problem hiding this comment.
The catch block only logs the error but doesn't handle it appropriately. Consider adding more specific error handling or rethrowing the exception to allow callers to handle it.
| Timber.e(e, "Error refreshing pending deposits") | |
| Timber.e(e, "Error refreshing pending deposits") | |
| throw e |
| return@let | ||
| } | ||
| snapshots.map { it.assetId }.distinct().forEach { | ||
| walletViewModel.findOrSyncAsset(it) |
There was a problem hiding this comment.
The sequential forEach call could result in multiple blocking database/network operations. Consider using parallel execution with async or batching these operations for better performance.
| walletViewModel.findOrSyncAsset(it) | |
| kotlinx.coroutines.coroutineScope { | |
| snapshots.map { it.assetId } | |
| .distinct() | |
| .map { assetId -> | |
| async { walletViewModel.findOrSyncAsset(assetId) } | |
| } | |
| .awaitAll() |
# Conflicts: # app/src/main/java/one/mixin/android/repository/Web3Repository.kt # app/src/main/java/one/mixin/android/ui/common/LoginVerifyBottomSheetDialogFragment.kt # app/src/main/java/one/mixin/android/ui/tip/wc/sessionproposal/SessionProposalPage.kt # app/src/main/java/one/mixin/android/ui/tip/wc/sessionproposal/SessionProposalViewModel.kt # app/src/main/java/one/mixin/android/ui/tip/wc/sessionrequest/SessionRequestViewModel.kt # app/src/main/java/one/mixin/android/ui/wallet/ClassicWalletFragment.kt # app/src/main/java/one/mixin/android/ui/wallet/PrivacyWalletFragment.kt # app/src/main/java/one/mixin/android/ui/wallet/TransactionsFragment.kt # app/src/main/java/one/mixin/android/ui/wallet/WalletFragment.kt
…eposit refresh logic