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

Feat/week2 - 2차 과제 완료 #8

Merged
merged 3 commits into from
Oct 27, 2021
Merged

Feat/week2 - 2차 과제 완료 #8

merged 3 commits into from
Oct 27, 2021

Conversation

i-colours-u
Copy link
Member

@i-colours-u i-colours-u commented Oct 17, 2021

📌 관련 이슈

#7
레이아웃은 기존 1차 과제에서 했던걸 동일하게 가져갑니다!
2차 과제에서 중점적으로 다뤘던 내용은,
Coordinator/Router/Module Factory 3개 구조를 활용해서 화면 전환을 관리하고 있습니다

Router를 통해서 Push/Present/Dismiss/Pop/SetRoot 를 처리하고,
인스턴스의 DI 처리는 Coordinator에서 처리합니다!

아직 각 뷰별로 Logic이 복잡한게 없어서 ViewModel로 따로 안빼고 VC에서 바로 Coordinator에게 전달하는 방식으로 작성중입니다! 이후에 뷰가 복잡해지면 따로 ViewModel로 분리하는형태로...

📌 PR Point

초기에 Coordinator 세팅과 관련한 파일들이 많으니 참고해주시고,
화면 전환에 대한 로직은 전부 Coordinator(Coordinator >> Application 폴더)에서 진행하고 있으니,
해당 과정이 필요하시면 그 부분을 참고해주시면 될 것 같습니다!!

화면전환 >> Router
뷰컨 인스턴스 생성 >> Module Factory
뷰컨의 이벤트 및 화면전환 제어 및 DI >> Coordinator
로 보시면 될 것 같아요!!

📌 참고 사항

이상한 점이나, 개선할 점 혹은 질문할 점이 있으면 편하게 마구마구 달아주세요!!!

@ezidayzi
Copy link
Member

스타 누르고 갑니다..

Copy link
Member

@heerucan heerucan left a comment

Choose a reason for hiding this comment

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

선배님. 왜? 이렇게까지 하시나요? 이러시는 이유가 있을 거 아니야;;;
지금 이러실 거면.. 쿠퍼티노 가셔라;
혹시 이거 제가 아는 swift 맞나요? 이거 뭐.. 송지프트 아닌가요?
저의 두뇌의 개발 지식 수용 범위 초과로 인해 방금. 뇌에 주름 하나 더 생겼네요;;;

@@ -14,6 +14,7 @@ class ApplicationCoordinator: BaseCoordinator {
private let coordinatorFactory: CoordinatorFactoryProtocol
private let moduleFactory: ModuleFactoryProtocol
private var launchInstructor = LaunchInstructor.configure()
private var wasLoggedIn: Bool = false
Copy link
Member

Choose a reason for hiding this comment

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

영어 잘하시네요. 역시 성대교육학과..

case .main:
self.runMainTabScene()
}
wasLoggedIn ? runMainTabScene() : runSigningScene()
Copy link
Member

Choose a reason for hiding this comment

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

와 40~45번 줄을 이해하고 있엇는데 지우고 40번 줄로 퉁친 걸 방금 발견했네요.
멋있습니다.

@@ -53,14 +48,14 @@ class ApplicationCoordinator: BaseCoordinator {

// MARK: - Run scenes
extension ApplicationCoordinator {

private func runSigningScene() {
var coordinator = self.coordinatorFactory.makeSigningCoordinator(
router: self.router,
coordinatorFactory: self.coordinatorFactory,
moduleFactory: self.moduleFactory)
coordinator.finishScene = { [unowned self, unowned coordinator] in
Copy link
Member

Choose a reason for hiding this comment

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

살아생전 unowned 를 써준 사람은 내 주변에 프송님이 처음이라 신기합니다.
weak를 안쓰고 unowned를 쓰셨네요.
그 이유에 대해서 50자 이내로 답변하시오.

Copy link
Member Author

Choose a reason for hiding this comment

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

unowned 랑 weak 의 가장 큰 차이점은 ARC에서 retain count가 증가하냐/ 안증가하냐가 가장 큰 차이일텐데여, unowned 참조는 참조한 객체가 메모리 해제 되었을때 nil로 변경이 안되는 점이 있어서, 참조한 객체의 생명주기가 더 길때 사용해요!!

여기서 저는 coordinator에서 나오는거 optional 처리 하기 싫고.. 무조건 coordinator는 끝까지 살아있어야 하기 때문에.. unowned로 사용했슴니다

@@ -14,6 +14,7 @@ class ApplicationCoordinator: BaseCoordinator {
private let coordinatorFactory: CoordinatorFactoryProtocol
private let moduleFactory: ModuleFactoryProtocol
private var launchInstructor = LaunchInstructor.configure()
Copy link
Member

Choose a reason for hiding this comment

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

Instructor가 뭔데? 강사에요?

Copy link
Member Author

Choose a reason for hiding this comment

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

launch 되었는지를 알려주는.. 클래스라서 그렇게 했던건데 ^^; 좀 오해의 소지가 있긴하네여

}
}
}

Copy link
Member

Choose a reason for hiding this comment

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

선배님 이 코드도 반복이 되니까 선배님이시라면 분명 더 간결하게 줄여보실 수 있을 거라고 이 까마득하게 모지리 후배가 감히.
제가.. 뭐라고 리뷰를 남겨봅니다.

Copy link
Member Author

Choose a reason for hiding this comment

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

제가 무지성으로 남발한 코드네요.... 좋은 지적 감삼다!!

@@ -52,3 +54,4 @@ struct UserDefaultOptional<T> {
set { self.storage.set(newValue, forKey: self.key.rawValue) }
Copy link
Member

Choose a reason for hiding this comment

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

아 또 저장까지 하신 걸까요?

Copy link
Member Author

@i-colours-u i-colours-u Oct 27, 2021

Choose a reason for hiding this comment

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

아뇨 그 UserDefaults 나중에 쓰는것두 있구, 현재 로그인 되었는지 안되었는지를 간단하게 UserDefulats로만 처리하고있는데
그냥 여기는 Property Wrapper로 한번 더 감싼 형태에요!

##### 제목은 최대 50 글자까지만 입력 ############## -> |

# 본문은 위에 작성
######## 본문은 한 줄에 최대 72 글자까지만 입력 ########################### -> |

# 꼬릿말은 아래에 작성: ex) #이슈 번호

# --- COMMIT END ---
# <타입> 리스트
# feat       sparkles               : 기능(새로운 기능)
# fix        beetle                 : 버그(버그 수정)
# refactor   hammer                 : refacetor 리팩토링
# style      art                    : 스타일( 코드 형식 변경, 로직 변경 없음)
# doc        pencil                 : 문서(문서 추가, 수정, 삭제)
# test       white_check_mark       : 테스트 코드 추가
# chore      moyai                  : 기타 변경사항
# ------------------
#     제목 첫 글자를 대문자로
#     제목은 명령문으로
#     제목 끝에 마침표(.) 금지
#     제목과 본문을 한 줄 띄워 분리하기
#     본문은 "어떻게" 보다 "무엇을", "왜"를 설명한다.
#     본문에 여러줄의 메시지를 작성할 땐 "-"로 구분
# ------------------
@i-colours-u i-colours-u merged commit cd26c72 into main Oct 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants