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.

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

지역설정 화면 구현 #25

Merged
merged 13 commits into from
Mar 20, 2024
Merged

지역설정 화면 구현 #25

merged 13 commits into from
Mar 20, 2024

Conversation

Jeon0976
Copy link
Member

@Jeon0976 Jeon0976 commented Mar 17, 2024

Motivation ⍰

  • 지역 화면 구현

Key Changes 🔑

  • APP

    1. HomeSceneDIContainer 수정
      • Infra Service를 주입 받을 수 있는 Dependencies 구조체 생성
      • DefaultRegionSettingUseCase 생성가능한 메서드 생성

    2.extetnsion으로 Domain / Data / Presentation 영역 분리

  • Domain / Entities

    1. MainRegion 엔티티 구현
      • region Enum, isSelected Bool을 보유하고 있습니다.
      • isSelected는 MainRegion 선택에 따라 SubRegion 값을 불러오기 위함입니다.
    2. SubRegion 엔티티 구현
      • region String, themeCount Int?을 보유하고 있습니다.
      • themeCount 값은 nil을 보장합니다. 이는 회원가입 로직간 count가 없는 화면이 있기 때문이며 해당 값을 불러오는 행위는 RegionsSettingUseCase 에서 진행합니다.
  • Domain / UseCase

    1. RegionSettingUseCase 구현
      • loadMainRegions Main 화면의 지역 값에 맞춰서 최초 화면을 불러오기 위해 파라미터 값을 받습니다. 현재 int형을 받지만, Region Enum 값 또는 String 값 중 효율적인 값으로 리팩토링 예정입니다.
      • loadSubRegions region enum 파라미터를 받으며 해당 값을 통해 fetchSubRegionsThemeCount 에서 count를 호출합니다. 여기서도 count를 안 불러도 되는 화면이 있기때문에 호출할지 말지 분류할 수 있는 값과 로직을 추가할 예정입니다.
      • fetchSubRegionsThemeCount 추후 repository 구현 시 연동 예정
  • Presentation / Coordinator

    • RegionSettingVC 구현 메서드 생성
  • Presentation / ViewModel

    • Actions 구조체 추가
    • RxSwift / RxCocoa를 활용하여 DataBinding
  • Presentation / ViewController, View

    • Navigation Title 초기값 설정 및 외부에서 주입 받게 수정 필요

To Reviewers 🙏🏻

  • BaseViewController에서 DataBinding 함수를 사용하면 ViewDidLoad 데이터 스트림 구독 시점이 적절하지 않습니다.

  • 각각의 create static 함수 내부에서 DataBinding 하는게 어떨가 싶습니다.

  • Main에서 RegionSetting 화면으로 좌측에서 나오도록 커스텀 Push 기능 구현 예정입니다.


Linked Issue 🔗

@Jeon0976 Jeon0976 self-assigned this Mar 17, 2024
@Jeon0976 Jeon0976 added the Feature New feature label Mar 17, 2024

}

func loadMainRegions(with isSelectedIndex: Int = 0) -> [MainRegion] {
Copy link
Member Author

Choose a reason for hiding this comment

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

index가 아닌 SubRegion "title" 값을 받아와서 해당 SubRegion title이 있는 MainRegion이 selected되도록 내부 코드를 수정하겠습니다.

Copy link
Member Author

Choose a reason for hiding this comment

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

파라미터를 위에 subRegion title로 받는 loadMainRegions 함수를 하나 더 만들겠습니다!

return fetchSubRegionsThemeCount(with: mainRegion)
}

private func fetchSubRegionsThemeCount(with mainRegion: Region) -> [SubRegion] {
Copy link
Member Author

Choose a reason for hiding this comment

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

main regions을 클릭 할 때마다 해당 main region 내부 subRegions count 개수를 불러들이는 api를 호출 할 예정인데,
해당 useCase를 호출 한 ViewModel에서 이미 한 번 호출했던 main region을 호출하면 api 호출 안하고 내부 캐쉬 데이터로 return 할 수 있도록 수정할 예정입니다. 또한

해당 캐쉬는 useCase가 메모리에서 초기화 되면 삭제될 수 있도록 수정할 예정입니다.

 - Delegate 패턴을 사용하지 않고 의존성 주입만으로 child VC에서 present VC로 data 주입
@Jeon0976
Copy link
Member Author

뒤로가기 아이콘, 애니메이션 추가, subRegion 클릭될때 animation 등 기타 UI 수정은 차차 진행하겠습니다.

Jeon0976 and others added 5 commits March 20, 2024 01:08
 '기타'에서 '서울 기타', '경기도 기타'... 순으로 string값 수정
- HomeUseCase -> DefaultHomeUseCase로 이름 변경
- HomeUseCaseInterface라는 이름으로 프로토콜을 만들고 DefaultHomeUseCase가 준수
- Base View에 layoutSubviews 오버라이딩 함수 추가
- 필요없는 파일 삭제
- 오토 레이아웃 view.safeAreaLayoutGuide에 맞춤
- separator inset zero로 설정
- mapButtonItem을 View Controller에 선언
- map button 이벤트를 bind view model에 적용
@@ -15,7 +15,9 @@ final class AppDIContainer {
}

func makeHomeSceneDIContainer() -> HomeSceneDIContainer {
return HomeSceneDIContainer()
let dependencise = HomeSceneDIContainer.Dependencies()
Copy link
Member

Choose a reason for hiding this comment

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

app 단에서 service를 주입하는 이유를 알 수 있을까요? 앱에서 직접적으로 data에 의존할 필요는 없을 거 같아서요
또한, VC에서 viewModel에 usecase를 주입하기 때문에 service를 주입할 필요가 없을 거 같습니다.

@@ -0,0 +1,172 @@
//
// RegionSettingViewController.swift
Copy link
Member

Choose a reason for hiding this comment

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

Home 파일 뎁스가 View, RegionSetting으로 나뉘어서 View안에 넣는게 어떤가 싶네요

with subRegion: String,
didSelect: @escaping RegionSettingViewModelDidSelectAction
) {
let vc = dependencies.makeRegionSettingViewController(with: subRegion, didSelect: didSelect)
Copy link
Member

Choose a reason for hiding this comment

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

subRange는 회원가입할 때 설정하거나 나중에 마이 페이지에서 변경할 수도 있을 거 같아서 상태 값을 UserDefault나 다른 방식으로 글로벌하게 저장하는게 어떤가 싶습니다.
서버로부터 따로 응답을 받아서 처리할 필요도 없기도 하고요

Copy link
Member Author

Choose a reason for hiding this comment

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

해당 상태값은 UserDefault로 저장하고 UserDefaults 관리는 Data영역에서 할 수 있도록 설정예정입니당

Copy link
Member

Choose a reason for hiding this comment

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

공유 자원이면 vc 간 데이터 전송 구현은 필요는 없을 거 같습니다.

Copy link
Member Author

Choose a reason for hiding this comment

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

아 맞네요! data영역 해당 repository만 만들면 모든게 다 원큐로 해결가능하네용
맞춰서 리펙토링 하겠습니다


func makeRegionSettingViewController(
with subRegion: String,
didSelect: @escaping RegionSettingViewModelDidSelectAction
Copy link
Member

Choose a reason for hiding this comment

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

home에서 subject를 넘겨주고 makeRegionSettingViewController에서 구독하고
cell 버튼 클릭 발생했을 때 값을 emit하는 방식으로 하는 게 어떤가 싶긴하네요

Copy link
Member Author

Choose a reason for hiding this comment

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

해당 방식도 괜찮을것같아요
기회가 될 때 subject로 리펙토링 해보겠습니다

@Minny27
Copy link
Member

Minny27 commented Mar 20, 2024

이전 develop merge 후 push하시고 merge하시면 될 거같습니다

고생하셨습니다. :)

@Jeon0976 Jeon0976 merged commit 6eb0eb0 into develop Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants