Merged
Conversation
테스트 내역: 환율정보가 티커에 반영되는지 확인합니다.
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR updates the exchange rate application for AllMarketTicker by moving price computation into the domain layer with a new Ticker entity, and it also refactors related test code. Key changes include:
- Renaming render object properties and updating view components to use the new naming (e.g. priceText, changePercentText).
- Refactoring repository and use-case implementations to filter tickers by USDT suffix and correctly apply exchange rate conversions.
- Rewriting tests to validate that the exchange rate is applied to ticker prices.
Reviewed Changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| TickerGridCell.swift | Updated property names from displayPriceText/displayChangePercentText to priceText/changePercentText for UI consistency. |
| AllMarketTickerView.swift | Renamed data properties from tickerCellRO to tickerCellRenderObjects to reflect updated domain models. |
| TickerSymbolComparators.swift, TickerNoneComparator.swift, TickerComparator.swift | Adjusted comparators to use the new Ticker entity instead of the old TickerCellRO. |
| TickerCellRO.swift | Replaced deprecated properties with the new ones and simplified the render object structure. |
| AllMarketTickerViewModel.swift | Refactored state management and cell creation methods to use the new Ticker entity and updated naming conventions. |
| AllMarketTickerTests.swift | Revised tests to assert that the exchange rate is properly applied to ticker prices. |
| StubExchangeRateRepository.swift, StubAllMarketTickerRepository.swift, FakeUserConfigurationRepository.swift | Updated testing repositories in line with the new Ticker and exchange rate logic. |
| DefaultAllMarketTickersUseCase.swift | Updated filtering and exchange rate conversion logic using the new use-case flow. |
| BinanceAllMarketTickersRepository.swift | Modified DTO conversion to create Ticker entities. |
| Domain Interface Files (TickerList.swift, Ticker.swift) | Introduced new entities and removed outdated ones. |
Comments suppressed due to low confidence (2)
Projects/Features/AllMarketTicker/Feature/Sources/AllMarketTickerViewModel.swift:321
- [nitpick] Consider renaming local variables (e.g. 'formatted_price_text', 'price_text') to use camelCase (e.g. 'formattedPriceText', 'priceText') in accordance with Swift naming conventions.
func createPriceText(_ price: Decimal, currencyType: CurrencyType) -> String {
Projects/Domain/Concrete/UseCase/DefaultAllMarketTickersUseCase.swift:53
- [nitpick] Local variable names such as 'only_usdt_tickers', 'user_config_repo', and 'fitted_to_exchange_rate_tickers' use snake_case; consider converting them to camelCase to ensure consistency with Swift best practices.
let only_usdt_tickers = allMarketTickersRepository.getTickers()...
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환율 적용위치 업데이트, Ticker엔티티 도입
Situation
기존의 환율적용 프로세스는 아래와 같습니다.
기능 응집도 저하
해당 구현 방식은 코인의 가격정보에 대한 기능이
ViewModel과UseCase로 분산되어 있어 이해하기 힘든 코드를 만들어 냈습니다.캡슐화 저하
뿐만아니라 코인 정보를 획득하는 함수는
getTickerList는 심볼에 USDT를 포함한 화폐만이 전달됨을 나타낼 수 없어 내구 구현사항에 대한 확인이 반드시 필요하게됩니다.Behavior
기능 응집
가격에 대한 정보는 반드시 존재해야하는 도메인 로직으로 분류할 수 있겠다고 판단했습니다. 따라서 가격 연산에 대한 책임을 UseCase객체로 격리했습니다.
캡슐화 깨짐 방지
getTickerList함수의 반환형으로TickerList타입을 정의했습니다. 해당 타입은 아래와 같이 티커 가격의 화폐정보를 보유하고 있습니다.해당 정보를 통해 화폐정보를 파악할 수 있음으로
UseCase의 캡슐화를 깨지 않도록 했습니다.Impact
해당 리팩토링을 통해 개선된 점은 다음과 같습니다.
테스트 코드 재작성
기존 테스트 내역은 아래와 같습니다.
비즈니스 로직과는 다소 먼 테스트이며, 오히려 테스트와 구현이 커플링되어 수정을 힘들게한다고 판단했습니다.
새롭케 테스트하는 기능은 아래와 같습니다.
테스트의 목적은 지정한 환율 정보가 코인의 가격정보에 올바르게 적용되는지 확인하는 코드입니다.
해당 테스트의 경우 분명한 비즈니스 로직을 테스트하고 있습니다.
추후 개선사항
UseCase로 한정시킬 것입니다.