Refactor: architecture cleanup (ViewModel decomposition + dead code removal)#69
Merged
Conversation
Behavior-preserving cleanup of the reader restore logic. - Extract the unified scroll-restore logic out of the ContentArea composable into two named functions: runScrollRestore (orchestrator) and awaitStableRestore (the watch-until-stable loop). The restore effect is now a one-line call. - Restructure the orchestrator's early returns into a single when/handled flow so each function stays within detekt's complexity limits; the loop's essential interrelated conditions carry a documented @Suppress("CyclomaticComplexMethod"). - Document every restore magic constant (poll cadence, stability window, percent tolerance, stability threshold, ghost-row percent, sentinel item size). - Add a doc block above ReaderProgressController's gating flags describing the transitions and the semi-independent axes. No runtime logic, thresholds, control flow, or ordering changed. Verified: ./gradlew testStandardDebugUnitTest detekt (349 tests, 0 failures, detekt clean).
ReaderViewModel aliased four ReaderProgressController fields as its own pass-through properties. Three (suppressAutoNavUntilUserInteraction, restoredScrollPercent, restoredProgressSnapshot) were never referenced; hasUserInteractedSinceLoad was read once internally and not by any UI code. Remove all four and route the single remaining read directly through the controller. The controller's own fields are unchanged (its tests use them), and the members still used by the reader UI (userHasDragged, restoreInProgress, currentLibraryItemId) stay. Verified: ./gradlew testStandardDebugUnitTest detekt (349 tests, 0 failures, detekt clean).
…nManager Move the resolved-image-dimension machinery out of ReaderViewModel into a new ImageDimensionManager: the fine-grained Compose state map, the prompt off-main cache flush, and the trailing-debounced content rebuild (with its jobs and pending/queued maps). ReaderViewModel keeps the ui-state-coupled content rewrite (updateCurrentContentImageDimensions / withResolvedImageDimensions) and passes it to the manager as a callback; it exposes the same public surface (resolvedImageDimensions, persistImageDimensions) by delegation, so callers and the reader UI are unchanged. resetState now calls manager.reset(), preserving the original behavior of leaving the db-flush job and pending map running. Verified: ./gradlew testStandardDebugUnitTest detekt (349 tests, 0 failures, detekt clean).
…entShaper WebContentLoader mixed ~130 lines of pure, stateless element shaping (long-strip detection, adjacent-image grouping, wide/double-page splitting) into its I/O and caching orchestration. Move those six functions and their two constants verbatim into a new stateless WebChapterElementShaper object; WebContentLoader now calls through it at the three existing sites. No behavior change. The moved code's pre-existing, already-baselined detekt findings (magic numbers, long lines, nesting/return-count on the grouping functions) are relocated in detekt-baseline.xml from WebContentLoader to WebChapterElementShaper — same findings, new home. Verified: ./gradlew testStandardDebugUnitTest detekt (349 tests, 0 failures, detekt clean).
…nager Move LibraryViewModel's selection state (_selectedItems, _selectionModeEnabled) and the toggle/select/deselect/group/all/enter/clear operations into a new LibrarySelectionManager. The ViewModel delegates and reads the manager's flows in its state combine; group- and select-all operations pass their ids in, keeping the manager decoupled from the library list and ui state. No behavior change. Verified: ./gradlew testStandardDebugUnitTest detekt (349 tests, 0 failures, detekt clean).
Move LibraryViewModel's pending-deletion state and the 5s undo-window logic (_pendingDeletion, the delete job, schedule/undo/commit/flush/removeImmediate) into a new LibraryDeletionCoordinator. The ViewModel delegates and reads the coordinator's pendingDeletion flow in its state combine; deletion errors surface back through a callback. No behavior change. Verified: ./gradlew testStandardDebugUnitTest detekt (349 tests, 0 failures, detekt clean).
Move LibraryViewModel's search/content-type/sort/status filter flows, their setters, and the pure filterAndSortItems function into a new LibraryFilters holder. The ViewModel exposes the flows by delegation and applies them via filters.apply() in its state combine. No behavior change. Verified: ./gradlew testStandardDebugUnitTest detekt (349 tests, 0 failures, detekt clean).
…tates Move LibraryViewModel's per-chapter download/cache badge state and its reconciliation — the queue observer, on-demand reconcile + auto-resume of incomplete downloads, and the semaphore-limited cache-state refresh — into a new LibraryDownloadStates. The ViewModel delegates the public reconcile/refresh methods, reads the manager's chapterCacheStates in its combine, and routes its three remaining setCacheState writes through it. No behavior change. Verified: ./gradlew testStandardDebugUnitTest detekt (349 tests, 0 failures, detekt clean).
With selection, deletion, filters, and download/cache-state extracted into their own collaborators, LibraryViewModel's class body is now under detekt's LargeClass threshold, so the suppression it carried is no longer needed. Verified: ./gradlew detekt (clean, no LargeClass).
Drop pre-Room content/session persistence that nothing reads anymore: saveParagraphs/loadParagraphs, saveLibraryItems, the currentUrl / currentTitle / scrollPosition properties, clearContent, and their now-orphaned keys. loadLibraryItems stays (LibraryRepository still uses it for one-time migration to Room). With the dead functions gone, PreferencesManager is under detekt's TooManyFunctions threshold, so its @Suppress is removed too. Verified: ./gradlew testStandardDebugUnitTest detekt (349 tests, 0 failures, detekt clean).
Delete confirmed-unused private members that were baselined rather than removed: nine dead regexes in TextUtils (one a duplicate of MULTIPLE_SPACES_REGEX), HtmlParser's unused WHITESPACE_REGEX, ReaderViewModel's unused MIN_SCROLL_OFFSET_DELTA_PX constant and isPlaceholderAtCurrentPosition (a duplicate of the live one in ReaderProgressController), and MangaBatSource's unused fetchChaptersFromApi fallback. Regenerated detekt-baseline.xml to drop the 13 now-stale entries. Verified: ./gradlew testStandardDebugUnitTest detekt (349 tests, 0 failures, detekt clean).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Decomposes the two largest ViewModels into focused collaborators and removes dead code that was grandfathered in the detekt baseline. No behavior change — every extraction is a verified line-faithful move.
Reader
Library
Web
Dead code
Verified per commit: ./gradlew testStandardDebugUnitTest detekt (349 tests, 0 failures, detekt clean).