refact :: [#117] 주말급식 리팩토링 #123
Conversation
|
Caution Review failedThe pull request is closed. Walkthrough이 풀 리퀘스트는 여러 파일에서의 구조체 및 클래스 수정 사항을 포함하고 있습니다. Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (7)
Projects/Domain/Sources/Entity/SchoolMeal/SchoolMealEntity.swift (1)
14-16: 문서화 개선 제안초기화 메서드에 각 매개변수의 의미와 예상되는 값 형식에 대한 문서 주석을 추가하면 좋을 것 같습니다.
+ /// 급식 번들을 초기화합니다. + /// - Parameter mealBundle: 급식 정보 배열 [(식사 종류, 식사 상세 정보)] + /// 예시: [("아침", MealEntityElement), ("점심", MealEntityElement)] public init(mealBundle: [(String, MealEntityElement)]) { self.mealBundle = mealBundle }Projects/Data/Sources/DTO/SchoolMeal/SchoolMealDTO.swift (1)
29-31: 식사 타입을 문자열로 표현한 것이 더 명확합니다.숫자 인덱스 대신 "조식", "중식", "석식"과 같은 명확한 문자열을 사용함으로써 코드의 가독성이 크게 향상되었습니다. 이는 도메인 용어를 더 잘 반영하며, 향후 유지보수도 더 쉬워질 것으로 보입니다.
추가적인 개선사항으로, 이러한 식사 타입 문자열을 열거형으로 정의하는 것을 고려해보시면 좋을 것 같습니다:
enum MealType: String { case breakfast = "조식" case lunch = "중식" case dinner = "석식" }Projects/Presentation/Sources/Base/BaseViewController.swift (1)
38-46: 뷰 레이아웃 생명주기 메서드의 책임 분리가 잘 되었습니다! 👍레이아웃 관련 메서드의 실행 시점을 명확하게 분리한 것이 좋은 설계입니다:
viewWillLayoutSubviews: 초기 뷰 계층 구조 설정viewDidLayoutSubviews: 네비게이션 바 및 데이터 기반 레이아웃 조정이러한 분리는 서브클래스에서 각 단계를 더 명확하게 재정의할 수 있게 해주며, UIKit의 뷰 레이아웃 생명주기를 더 잘 활용하고 있습니다.
Projects/Modules/DesignSystem/Sources/Component/Alert/PiCKAlert.swift (2)
70-71: 모달 프레젠테이션 스타일이 적절하게 설정되었습니다.크로스 디졸브 트랜지션과 오버 풀스크린 프레젠테이션은 알림 UI에 적합한 선택입니다.
접근성 향상을 위해 다음과 같은 개선을 고려해보세요:
self.modalTransitionStyle = .crossDissolve self.modalPresentationStyle = .overFullScreen +self.view.accessibilityViewIsModal = true +self.backgroundView.accessibilityLabel = titleText
Line range hint
18-18: 클로저 이름을 더 일반적인 용도로 변경하는 것이 좋겠습니다.
clickLogout이라는 이름은 특정 기능에 한정되어 있습니다. 알림이 다양한 상황에서 재사용될 수 있도록 더 일반적인 이름을 사용하는 것이 좋겠습니다.다음과 같이 변경을 제안합니다:
-public var clickLogout: () -> Void +public var onConfirm: () -> Void이에 따라
bindActions()메서드도 수정이 필요합니다:confirmButton.rx.tap .bind { [weak self] in self?.dismiss(animated: true) { - self?.clickLogout() + self?.onConfirm() } }.disposed(by: disposeBag)Projects/Presentation/Sources/Scene/SchoolMeal/SchoolMealViewController.swift (1)
61-70: 튜플 대신 명시적 타입 사용 권장현재 튜플을 사용하여 데이터를 전달하고 있는데, 이는 코드의 가독성과 타입 안전성을 저하시킬 수 있습니다. 명시적인 타입을 사용하는 것이 더 좋은 방법일 것 같습니다.
다음과 같은 리팩토링을 제안합니다:
- output.schoolMealData + // MealTimeData 타입 정의 + struct MealTimeData { + let mealTime: String + let mealInfo: MealEntityElement + } + // ViewModel에서 반환하는 타입을 변경 + output.schoolMealData .asObservable() .bind(to: schoolMealCollectionView.rx.items( cellIdentifier: SchoolMealCollectionViewCell.identifier, cellType: SchoolMealCollectionViewCell.self - )) { _, item, cell in + )) { _, mealData, cell in cell.setup( - mealTime: item.0, - menu: item.1.menu, - kcal: item.1.kcal + mealTime: mealData.mealTime, + menu: mealData.mealInfo.menu, + kcal: mealData.mealInfo.kcal ) }Projects/Presentation/Sources/Scene/Home/HomeViewController.swift (1)
313-322: setupViewType 메서드의 리팩토링이 잘 이루어졌습니다기존의 switch 문을 boolean 로직으로 단순화한 것은 좋은 개선입니다. 코드가 더 간결하고 이해하기 쉬워졌습니다.
몇 가지 추가적인 개선 포인트를 제안드립니다:
- 상수로 분리된 isTimeTable 변수의 활용이 좋습니다
- 뷰의 가시성 설정이 더 명확해졌습니다
스냅킷 제약조건 설정 부분을 별도 메서드로 분리하는 것을 고려해보세요:
+private func updateMainStackViewConstraints(height: CGFloat) { + mainStackView.snp.remakeConstraints { + $0.height.equalTo(height) + } +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (10)
Projects/Data/Sources/DTO/SchoolMeal/SchoolMealDTO.swift(1 hunks)Projects/Domain/Sources/Entity/SchoolMeal/SchoolMealEntity.swift(1 hunks)Projects/Modules/DesignSystem/Sources/Component/Alert/PiCKAlert.swift(1 hunks)Projects/Presentation/Sources/Base/BaseViewController.swift(1 hunks)Projects/Presentation/Sources/Scene/AllTab/AllTabViewController.swift(0 hunks)Projects/Presentation/Sources/Scene/Home/HomeViewController.swift(3 hunks)Projects/Presentation/Sources/Scene/Home/HomeViewModel.swift(2 hunks)Projects/Presentation/Sources/Scene/OnBoarding/OnBoardingViewController.swift(0 hunks)Projects/Presentation/Sources/Scene/SchoolMeal/SchoolMealViewController.swift(1 hunks)Projects/Presentation/Sources/Scene/SchoolMeal/SchoolMealViewModel.swift(2 hunks)
💤 Files with no reviewable changes (2)
- Projects/Presentation/Sources/Scene/AllTab/AllTabViewController.swift
- Projects/Presentation/Sources/Scene/OnBoarding/OnBoardingViewController.swift
🔇 Additional comments (3)
Projects/Data/Sources/DTO/SchoolMeal/SchoolMealDTO.swift (1)
22-22: 속성 선언 간소화가 잘 되었습니다!
Swift의 간결한 문법을 활용하여 코드를 더욱 깔끔하게 만들었습니다.
Projects/Presentation/Sources/Scene/SchoolMeal/SchoolMealViewModel.swift (2)
26-26: 데이터 구조가 단순화되었습니다.
Int 타입의 인덱스를 제거함으로써 데이터 구조가 더 명확해졌습니다. 이는 주말 급식 기능의 유지보수성을 향상시킬 것으로 보입니다.
29-29: private 프로퍼티의 데이터 타입이 일관되게 변경되었습니다.
Output 구조체의 변경사항과 일치하도록 BehaviorRelay의 타입이 적절히 수정되었습니다.
| @@ -9,9 +9,9 @@ public struct SchoolMealEntity { | |||
| } | |||
|
|
|||
| public struct SchoolMealEntityElement { | |||
| public let mealBundle: [(Int, String, MealEntityElement)] | |||
| public let mealBundle: [(String, MealEntityElement)] | |||
There was a problem hiding this comment.
💡 Codebase verification
데이터 구조 마이그레이션이 완료되지 않았습니다
HomeSchoolMealView에서 여전히 이전 데이터 구조 [(Int, String, MealEntityElement)]를 사용하고 있습니다. 다음 위치에서 업데이트가 필요합니다:
Projects/Presentation/Sources/Scene/Home/View/HomeSchoolMealView.swift:schoolMealDataBehaviorRelay의 타입을[(String, MealEntityElement)]로 변경setup메서드의 파라미터 타입을[(String, MealEntityElement)]로 변경bind메서드 내 cell setup 로직을 새로운 구조에 맞게 수정 (item.1대신item.0사용)
🔗 Analysis chain
데이터 구조 단순화 검증 필요
mealBundle의 타입이 [(Int, String, MealEntityElement)]에서 [(String, MealEntityElement)]로 변경되었습니다. 이는 좋은 리팩토링이지만, 모든 의존성이 올바르게 업데이트되었는지 확인이 필요합니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify all usages of SchoolMealEntityElement have been updated
# Search for any remaining references to the old tuple structure
rg -l "[(]Int,\s*String,\s*MealEntityElement[)]"
# Find all files that might be using this structure
fd -e swift -x grep -l "SchoolMealEntityElement"
Length of output: 296
Script:
#!/bin/bash
# Check the content of HomeSchoolMealView.swift which still has references to old structure
cat "Projects/Presentation/Sources/Scene/Home/View/HomeSchoolMealView.swift"
# Check SchoolMealDTO.swift for any related changes
cat "Projects/Data/Sources/DTO/SchoolMeal/SchoolMealDTO.swift"
Length of output: 2909
| .bind { data in | ||
| self.schoolMealData.accept(data.meals.mealBundle) | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
셀 크기가 고정값으로 설정되어 있습니다
SchoolMealViewController에서 CollectionView의 셀 크기가 width: view.frame.width - 48, height: 154로 고정되어 있습니다. 동적 크기 조정이 필요한 경우 다음 사항들을 고려해야 합니다:
- 메뉴 텍스트 길이에 따른 셀 높이 동적 조정
- UICollectionViewDelegateFlowLayout 프로토콜 구현
- estimatedItemSize 또는 sizeForItemAt 메서드 활용
🔗 Analysis chain
RxSwift 바인딩이 개선되었습니다.
subscribe(onNext:) 대신 bind를 사용하여 코드가 더 간결해졌습니다. 하지만 동적 셀 크기 조정과 관련된 요구사항이 반영되었는지 확인이 필요합니다.
셀 크기 조정 관련 코드를 확인하기 위해 다음 스크립트를 실행합니다:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: 셀 크기 조정 관련 코드 확인
# SchoolMealViewController에서 셀 크기 조정 관련 코드 검색
rg -t swift "cell.*size|size.*cell"
# UICollectionViewDelegateFlowLayout 구현 확인
ast-grep --pattern 'class $_ : $_ {
$$$
collectionView($_, layout: UICollectionViewLayout, sizeForItemAt: IndexPath) {
$$$
}
$$$
}'
Length of output: 183
Script:
#!/bin/bash
# SchoolMealViewController 파일 내용 확인
fd "SchoolMealViewController.swift" --exec cat {}
# UICollectionView 관련 설정 검색
rg -t swift "UICollectionView.*delegate|collectionView.*delegate"
# 셀 크기 관련 코드 검색 (더 넓은 범위)
rg -t swift "itemSize|sizeForItem|estimatedItem"
Length of output: 5967
| output.schoolMealData | ||
| .asObservable() | ||
| .bind(to: schoolMealCollectionView.rx.items( | ||
| cellIdentifier: SchoolMealCollectionViewCell.identifier, | ||
| cellType: SchoolMealCollectionViewCell.self | ||
| )) { _, item, cell in | ||
| cell.setup( | ||
| mealTime: item.1, | ||
| menu: item.2.menu, | ||
| kcal: item.2.kcal | ||
| mealTime: item.0, | ||
| menu: item.1.menu, | ||
| kcal: item.1.kcal |
There was a problem hiding this comment.
주말 급식 셀 크기 동적 조정 구현 필요
PR 목표에서 언급된 주말 급식 셀 크기의 동적 조정이 구현되지 않았습니다. 현재 collectionViewFlowLayout에서 고정된 크기를 사용하고 있습니다.
다음과 같은 구현을 제안합니다:
- private lazy var collectionViewFlowLayout = UICollectionViewFlowLayout().then {
- $0.itemSize = .init(
- width: self.view.frame.width - 48,
- height: 154
- )
- $0.minimumLineSpacing = 20
- }
+ private lazy var collectionViewFlowLayout = UICollectionViewFlowLayout().then {
+ $0.estimatedItemSize = UICollectionViewFlowLayout.automaticSize
+ $0.minimumLineSpacing = 20
+ }
+ // SchoolMealCollectionViewCell에서 구현
+ override func preferredLayoutAttributesFitting(
+ _ layoutAttributes: UICollectionViewLayoutAttributes
+ ) -> UICollectionViewLayoutAttributes {
+ let targetSize = CGSize(
+ width: layoutAttributes.frame.width,
+ height: UIView.layoutFittingCompressedSize.height
+ )
+ layoutAttributes.frame.size = contentView.systemLayoutSizeFitting(
+ targetSize,
+ withHorizontalFittingPriority: .required,
+ verticalFittingPriority: .fittingSizeLevel
+ )
+ return layoutAttributes
+ }Committable suggestion skipped: line range outside the PR's diff.
| @@ -62,7 +62,7 @@ public class HomeViewModel: BaseViewModel, Stepper { | |||
| let applyStatusData: Signal<HomeApplyStatusEntity> | |||
| let weekendMealPeriodData: Signal<WeekendMealPeriodEntity> | |||
| let timetableData: Driver<[TimeTableEntityElement]> | |||
| let schoolMealData: Driver<[(Int, String, MealEntityElement)]> | |||
| let schoolMealData: Driver<[(String, MealEntityElement)]> | |||
There was a problem hiding this comment.
💡 Codebase verification
HomeSchoolMealView에서 타입 불일치 발견
HomeViewModel에서 변경된 schoolMealData: [(String, MealEntityElement)] 타입이 HomeSchoolMealView에서 여전히 [(Int, String, MealEntityElement)]로 사용되고 있습니다. 다음 파일들의 수정이 필요합니다:
Projects/Presentation/Sources/Scene/Home/View/HomeSchoolMealView.swift- private let schoolMealData 프로퍼티의 타입을
[(String, MealEntityElement)]로 변경 - setup 메서드의 파라미터 타입을
[(String, MealEntityElement)]로 변경 - bind 메서드에서 cell.setup 호출 시 item.1 대신 item.0 사용
- private let schoolMealData 프로퍼티의 타입을
🔗 Analysis chain
데이터 구조 단순화 승인
튜플에서 Int 타입을 제거하여 데이터 구조를 단순화한 것은 좋은 변경사항입니다. 이는 주말 급식 리팩토링의 목적에 잘 부합하며, DTO와 Entity의 변경사항과도 일관성이 있습니다.
다음 스크립트를 실행하여 HomeViewModel의 schoolMealData를 사용하는 모든 곳이 새로운 타입으로 업데이트되었는지 확인하겠습니다:
Also applies to: 79-79
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: HomeViewModel.schoolMealData의 사용처 확인
# HomeViewModel.schoolMealData를 사용하는 모든 파일 검색
rg -l "schoolMealData.*\(Int,\s*String,\s*MealEntityElement\)" --type swift
# View에서의 바인딩 코드 검색
rg "bind\(to:.*schoolMealData\)" --type swift -A 3
Length of output: 201
Script:
#!/bin/bash
# HomeSchoolMealView.swift 파일의 내용을 확인하여 타입 변경이 필요한지 검사
cat "Projects/Presentation/Sources/Scene/Home/View/HomeSchoolMealView.swift"
# schoolMealData 관련 바인딩 코드를 더 넓은 컨텍스트로 검색
rg "schoolMealData" --type swift -B 2 -A 2
Length of output: 9142
| if height != 0 { | ||
| owner.timeTableHeight.accept(height) | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
timeTableHeight 바인딩 로직 개선 제안
height가 0이 아닐 때만 timeTableHeight를 업데이트하는 로직이 추가되었습니다. 하지만 이 접근 방식에는 몇 가지 고려사항이 있습니다:
- height가 0인 경우의 처리가 명시적이지 않습니다
- 이전 값이 유지되는 상황에 대한 고려가 필요합니다
다음과 같은 개선을 제안드립니다:
-if height != 0 {
- owner.timeTableHeight.accept(height)
-}
+let newHeight = height != 0 ? height : owner.timeTableHeight.value
+owner.timeTableHeight.accept(newHeight)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if height != 0 { | |
| owner.timeTableHeight.accept(height) | |
| } | |
| let newHeight = height != 0 ? height : owner.timeTableHeight.value | |
| owner.timeTableHeight.accept(newHeight) | |
| @@ -25,7 +23,7 @@ public class HomeViewController: BaseViewController<HomeViewModel> { | |||
| width: self.view.frame.width, | |||
| height: 0 | |||
| ) | |||
| private lazy var timeTableHeight = BehaviorRelay<CGFloat>(value: 0) | |||
| private lazy var timeTableHeight = BehaviorRelay<CGFloat>(value: 100) | |||
There was a problem hiding this comment.
💡 Codebase verification
timeTableHeight 초기값 변경은 의도적이지 않은 것으로 보입니다
HomeViewModel에서 timeTableHeight는 0으로 초기화되어 시간표 데이터에 따라 동적으로 계산됩니다 (데이터 개수 * 55). ViewController에서 100으로 하드코딩된 초기값은 이 로직과 충돌할 수 있습니다.
Projects/Presentation/Sources/Scene/Home/HomeViewController.swift: timeTableHeight 초기값을 0으로 되돌려야 합니다Projects/Presentation/Sources/Scene/Home/HomeViewModel.swift: ViewModel에서 실제 높이를 계산하여 설정하는 로직이 이미 구현되어 있습니다
🔗 Analysis chain
timeTableHeight의 초기값 변경에 대한 검토 필요
timeTableHeight의 초기값이 0에서 100으로 변경되었습니다. 이는 시간표 뷰의 초기 렌더링에 영향을 줄 수 있습니다.
초기값 변경이 의도적인지 확인이 필요합니다. 다음 스크립트로 다른 뷰컨트롤러에서의 영향도를 확인해보세요:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# timeTableHeight 사용 패턴 검색
rg -A 3 "timeTableHeight.*value.*=.*0" --type swift
Length of output: 53
Script:
#!/bin/bash
# timeTableHeight 관련 패턴을 더 넓게 검색
rg "timeTableHeight" --type swift -A 5 -B 5
Length of output: 8441
Script:
#!/bin/bash
# BehaviorRelay 초기화 패턴 검색
rg "BehaviorRelay.*value.*\d+" --type swift
Length of output: 1587
Script:
#!/bin/bash
# HomeViewController에서 timeTableHeight 사용 확인
ast-grep --pattern 'timeTableHeight.accept($$$)'
Length of output: 50
개요
작업사항
UI
close #117
Summary by CodeRabbit
새로운 기능
버그 수정
문서화
리팩토링
HomeViewController및SchoolMealViewModel의 데이터 바인딩 로직이 간소화되었습니다.PiCKAlert클래스의 초기화 메서드가 개선되었습니다.BaseViewController의 레이아웃 관리 로직이 개선되었습니다.