Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refact :: [#117] 주말급식 리팩토링 #123

Merged
merged 4 commits into from
Nov 19, 2024
Merged

refact :: [#117] 주말급식 리팩토링 #123

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4e6b295
refact :: [#117] 주말급식 로직 리팩토링
cyj513 Nov 18, 2024
f512498
chore :: 자잘한 수정
cyj513 Nov 18, 2024
ff2bb36
chore :: 뷰 로드 시점 분리
cyj513 Nov 18, 2024
abe5cc9
chore :: [#117] 튜플 값 변경
cyj513 Nov 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions Projects/Data/Sources/DTO/SchoolMeal/SchoolMealDTO.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,16 @@ extension SchoolMealDTO {
}

struct SchoolMealDTOElement: Decodable {
public let breakfast: MealDTOElement
public let lunch: MealDTOElement
public let dinner: MealDTOElement
public let breakfast, lunch, dinner: MealDTOElement
}

extension SchoolMealDTOElement {
func toDomain() -> SchoolMealEntityElement {
return .init(
mealBundle: [
(0, "조식", breakfast.toDomain()),
(1, "중식", lunch.toDomain()),
(2, "석식", dinner.toDomain())
("조식", breakfast.toDomain()),
("중식", lunch.toDomain()),
("석식", dinner.toDomain())
]
)
}
Expand Down
4 changes: 2 additions & 2 deletions Projects/Domain/Sources/Entity/SchoolMeal/SchoolMealEntity.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ public struct SchoolMealEntity {
}

public struct SchoolMealEntityElement {
public let mealBundle: [(Int, String, MealEntityElement)]
public let mealBundle: [(String, MealEntityElement)]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

데이터 구조 마이그레이션이 완료되지 않았습니다

HomeSchoolMealView에서 여전히 이전 데이터 구조 [(Int, String, MealEntityElement)]를 사용하고 있습니다. 다음 위치에서 업데이트가 필요합니다:

  • Projects/Presentation/Sources/Scene/Home/View/HomeSchoolMealView.swift:
    • schoolMealData BehaviorRelay의 타입을 [(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


public init(mealBundle: [(Int, String, MealEntityElement)]) {
public init(mealBundle: [(String, MealEntityElement)]) {
self.mealBundle = mealBundle
}
}
Expand Down
2 changes: 2 additions & 0 deletions Projects/Modules/DesignSystem/Sources/Component/Alert/PiCKAlert.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ public class PiCKAlert: UIViewController {
}
self.clickLogout = clickLogout
super.init(nibName: nil, bundle: nil)
self.modalTransitionStyle = .crossDissolve
self.modalPresentationStyle = .overFullScreen
}
required init?(coder: NSCoder) {
fatalError("init(coder:) has not been implemented")
Expand Down
7 changes: 5 additions & 2 deletions Projects/Presentation/Sources/Base/BaseViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,13 @@ open class BaseViewController<ViewModel: BaseViewModel>: UIViewController, UIGes
viewWillAppearRelay.accept(())
bindAction()
}
open override func viewDidLayoutSubviews() {
super.viewDidLayoutSubviews()
open override func viewWillLayoutSubviews() {
super.viewWillLayoutSubviews()
addView()
setLayout()
}
open override func viewDidLayoutSubviews() {
super.viewDidLayoutSubviews()
configureNavgationBarLayOutSubviews()
setLayoutData()
}
Expand Down
2 changes: 0 additions & 2 deletions Projects/Presentation/Sources/Scene/AllTab/AllTabViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,6 @@ public class AllTabViewController: BaseViewController<AllTabViewModel> {
) {
owner.logoutRelay.accept(())
}
alert.modalPresentationStyle = .overFullScreen
alert.modalTransitionStyle = .crossDissolve
owner.present(alert, animated: true)
}.disposed(by: disposeBag)
}
Expand Down
36 changes: 12 additions & 24 deletions Projects/Presentation/Sources/Scene/Home/HomeViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import Then
import RxSwift
import RxCocoa

import Starscream

import Core
import Domain
import DesignSystem
Expand All @@ -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)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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

private lazy var schoolMealHeight = BehaviorRelay<CGFloat>(value: 0)

private let scrollView = UIScrollView().then {
Expand Down Expand Up @@ -242,9 +240,7 @@ public class HomeViewController: BaseViewController<HomeViewModel> {
output.timeTableHeight.asObservable()
.withUnretained(self)
.bind { owner, height in
if height == 0 {
owner.timeTableHeight.accept(100)
} else {
if height != 0 {
owner.timeTableHeight.accept(height)
}

Comment on lines +243 to 246
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

timeTableHeight 바인딩 로직 개선 제안

height가 0이 아닐 때만 timeTableHeight를 업데이트하는 로직이 추가되었습니다. 하지만 이 접근 방식에는 몇 가지 고려사항이 있습니다:

  1. height가 0인 경우의 처리가 명시적이지 않습니다
  2. 이전 값이 유지되는 상황에 대한 고려가 필요합니다

다음과 같은 개선을 제안드립니다:

-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.

Suggested change
if height != 0 {
owner.timeTableHeight.accept(height)
}
let newHeight = height != 0 ? height : owner.timeTableHeight.value
owner.timeTableHeight.accept(newHeight)

Expand Down Expand Up @@ -314,24 +310,16 @@ public class HomeViewController: BaseViewController<HomeViewModel> {
}

private func setupViewType(type: HomeViewType) {
switch type {
case .timeTable:
self.todaysLabel.text = "오늘의 시간표"
self.schoolMealView.isHidden = true
self.timeTableView.isHidden = false

mainStackView.snp.remakeConstraints {
$0.height.equalTo(self.timeTableHeight.value)
}

case .schoolMeal:
self.todaysLabel.text = "오늘의 급식"
self.timeTableView.isHidden = true
self.schoolMealView.isHidden = false

mainStackView.snp.remakeConstraints {
$0.height.equalTo(self.schoolMealHeight.value)
}
let isTimeTable = (type == .timeTable)
self.todaysLabel.text = isTimeTable ? "오늘의 시간표" : "오늘의 급식"

self.timeTableView.isHidden = !isTimeTable
self.schoolMealView.isHidden = isTimeTable

let height = isTimeTable ? self.timeTableHeight.value : self.schoolMealHeight.value

mainStackView.snp.remakeConstraints {
$0.height.equalTo(height)
}
}

Expand Down
4 changes: 2 additions & 2 deletions Projects/Presentation/Sources/Scene/Home/HomeViewModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)]>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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 사용
🔗 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

let noticeListData: Signal<NoticeListEntity>
let selfStudyData: Driver<SelfStudyEntity>
let outingPassData: Signal<OutingPassEntity>
Expand All @@ -76,7 +76,7 @@ public class HomeViewModel: BaseViewModel, Stepper {
private let applyStatusData = PublishRelay<HomeApplyStatusEntity>()
private let weekendMealPeriodData = PublishRelay<WeekendMealPeriodEntity>()
private let timetableData = BehaviorRelay<[TimeTableEntityElement]>(value: [])
private let schoolMealData = BehaviorRelay<[(Int, String, MealEntityElement)]>(value: [])
private let schoolMealData = BehaviorRelay<[(String, MealEntityElement)]>(value: [])
private let outingPassData = PublishRelay<OutingPassEntity>()
private let noticeListData = PublishRelay<NoticeListEntity>()
private let selfStudyData = BehaviorRelay<SelfStudyEntity>(value: [])
Expand Down
3 changes: 0 additions & 3 deletions Projects/Presentation/Sources/Scene/OnBoarding/OnBoardingViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,6 @@ public class OnboardingViewController: BaseViewController<OnboardingViewModel> {
) {
print("click")
}
alert.modalTransitionStyle = .crossDissolve
alert.modalPresentationStyle = .overFullScreen

owner.present(alert, animated: true)
}.disposed(by: disposeBag)
}
Expand Down
9 changes: 5 additions & 4 deletions Projects/Presentation/Sources/Scene/SchoolMeal/SchoolMealViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,16 @@ public class SchoolMealViewController: BaseViewController<SchoolMealViewModel> {
)
let output = viewModel.transform(input: input)

output.schoolMealData.asObservable()
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
Comment on lines +61 to +70
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

주말 급식 셀 크기 동적 조정 구현 필요

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.

)
}.disposed(by: disposeBag)

Expand Down
10 changes: 5 additions & 5 deletions Projects/Presentation/Sources/Scene/SchoolMeal/SchoolMealViewModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ public class SchoolMealViewModel: BaseViewModel, Stepper {
let schoolMealDate: Observable<String>
}
public struct Output {
let schoolMealData: Driver<[(Int, String, MealEntityElement)]>
let schoolMealData: Driver<[(String, MealEntityElement)]>
}

private let schoolMealData = BehaviorRelay<[(Int, String, MealEntityElement)]>(value: [])
private let schoolMealData = BehaviorRelay<[(String, MealEntityElement)]>(value: [])

public func transform(input: Input) -> Output {
input.schoolMealDate
Expand All @@ -37,9 +37,9 @@ public class SchoolMealViewModel: BaseViewModel, Stepper {
return .never()
}
}
.subscribe(onNext: { [weak self] data in
self?.schoolMealData.accept(data.meals.mealBundle)
})
.bind { data in
self.schoolMealData.accept(data.meals.mealBundle)
}
Comment on lines +40 to +42
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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

.disposed(by: disposeBag)

return Output(schoolMealData: schoolMealData.asDriver())
Expand Down