[CVW-041] AllMarketTicker화면 ViewModel코드 리팩토링 및 예시앱 정상화#53
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the AllMarketTicker ViewModel and synchronizes the Example app for smoother testing and clearer sorting logic. Key changes include:
- Refactored access control and updated naming conventions in test doubles and repositories.
- Updated sorting logic by unifying comparator protocols and adjusting sort selection state management.
- Revised DI registrations and app bootstrapping to improve testability.
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Projects/Features/AllMarketTicker/Testing/MockWebSocketManagementHelper.swift | Made the helper class public and final to align with updated access control requirements. |
| Projects/Features/AllMarketTicker/Testing/MockAlertShooter.swift | Updated the class to public final with an explicit initializer. |
| Projects/Features/AllMarketTicker/Testing/FakeUserConfigurationRepository.swift | Revised access control and method signatures; noted a typo in a method name. |
| Projects/Features/AllMarketTicker/Testing/FakeI18NManager.swift | Adjusted public access modifiers and initialization for consistency. |
| Projects/Features/AllMarketTicker/Testing/FakeAllMarketTickerUseCase.swift | Added a fake use case implementation to facilitate logic testing. |
| Projects/Features/AllMarketTicker/Feature/Sources/View/AllMarketTickerView.swift | Updated sort selection handling and UI frame adjustments to improve layout consistency. |
| Projects/Features/AllMarketTicker/Feature/Sources/Model/* | Unified comparator protocols and revised rendering objects for sort selection. |
| Projects/Features/AllMarketTicker/Feature/Sources/AllMarketTickerViewModel.swift | Refactored state management and sorting logic; removed the main-thread dispatch which might impact UI updates. |
| Projects/Features/AllMarketTicker/Example/Sources/DI/Assemblies.swift & App.swift | Updated dependency injection and app assembling for the example application. |
Comments suppressed due to low confidence (1)
Projects/Features/AllMarketTicker/Feature/Sources/AllMarketTickerViewModel.swift:260
- Removing '.receive(on: DispatchQueue.main)' may lead to UI updates being processed off the main thread, which can cause inconsistent UI behavior. Consider reinstating it to ensure UI state updates occur on the main thread.
.receive(on: DispatchQueue.main)
Projects/Features/AllMarketTicker/Testing/FakeUserConfigurationRepository.swift
Outdated
Show resolved
Hide resolved
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
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.
변경된 점
AllMarketTicker화면 ViewModel코드 리팩토링
기존코드의 경우 정렬 로직을 결정하는 부분이 복잡했습니다.
특히
State구조체에 View와는 무관한 상태가 있어 해당 부분을 제거하였습니다.예시앱 정상화
AllMarketTickerExampleApp을 수정하여TestDoubles객체들로 로직테스트가 가능하도록 설정했습니다.