7주차 세미나 과제 #4
7주차 세미나 과제 #4
Conversation
hooni0918
requested review from
HELLOHIDI,
oyslucy,
thingineeer,
JinUng41,
kim-seonwoo and
Nya128
| func mainViewController() -> MainViewController? { | ||
| return viewControllers[0] as? MainViewController | ||
| } | ||
|
|
There was a problem hiding this comment.
바로 MainViewController를 타입 캐스팅 없이 할 수도 있을 것 같습니다!
There was a problem hiding this comment.
오 좋은 아이디어군요! 굳이 하드코딩을 할필요가 없었네요! 감사합니다
| func scrollViewDidScroll(_ scrollView: UIScrollView, navigationController: UINavigationController?) { | ||
| let offset = scrollView.contentOffset.y | ||
| let isScrollingDown = offset > 0 | ||
| navigationController?.setNavigationBarHidden(isScrollingDown, animated: true) | ||
| } |
There was a problem hiding this comment.
저는 이부분을 UI관련 로직이라고 판단하여 VC에서 처리해주었습니다!
어느정도 까지 VM에서 처리하면 좋을지.. 의견 듣고 싶습니다!
There was a problem hiding this comment.
저도 이부분을 많이 고민하긴 했습니다. 해당 메서드의 정확한 기능은 스크롤 상태에 따라 setNavigationBarHidden을 하는것이여서 UI로직이 아닌가? 싶었어요.
하지만 비즈니스 로직의 기준은 규칙에 의해서 가공하여 사용하고 코드를 재사용할수 있는가 이라고 생각합니다.
이번 과제가 앞으로 확장될 가능성은 없겟지만 만약 이와 비슷한 앱을 구현한다면 상단 segment에 따라서 더 많은 VC들이 늘어날것이고 해당 VC들은 메인VC에 맞춰 setNavigationBarHidden이 일어날것이라 생각했어요.
다라서 재활용이 가능할것이라 생각하여 해당 메서드를 VM에 넣게 되었습니다. 물론 이번 과제는 UI로직에 넣어도 무방할것같아요!!
매서운질문 감사합니다!!
| import RxSwift | ||
| import RxCocoa |
| @objc func backToMain() { | ||
| let mainVC = RootCollectionViewController() | ||
| mainVC.modalPresentationStyle = .fullScreen | ||
| self.present(mainVC, animated: true, completion: nil) | ||
| } |
There was a problem hiding this comment.
네비게이션 스택에 쌓지 않고
새로운 네비게이션 스택을 생성하고 rootViewController로 RootCollectionViewController()로 바꾸어 전환 하는 방법은
어떻게 생각하시나요~?
There was a problem hiding this comment.
로그인 이후 onboarding 화면으로 다시 돌아갈일이 없을것같아 일부로 네비게이션 스택으로 화면을 전환하지는 않았습니다.
사실 위에서 말한 제 논리와 어긋나는 부분이 있는데 RootCollectionViewController 에 진입할때 어찌되었든 네비게이션 스택을 새로 생성해야했는데 그러지 않았네요 정말 매서운 질문이였습니다!
| idTextFieldView.rx.text.orEmpty | ||
| .bind(to: viewModel.idInput) | ||
| .disposed(by: disposeBag) | ||
|
|
There was a problem hiding this comment.
orEmpty가 뭔지 몰라 찾아봤는데, 옵셔널을 처리하기 위한 Operator이군요?
덕분에 또 하나 배웠습니다. 감사합니다~
There was a problem hiding this comment.
저도 몰랏는데 또 배웟습니다. 덕분에 또 복기를 하네요 영광입니다 킹진웅짱진웅갓진웅
| @discardableResult | ||
| private func validateNickname(_ nickname: String) -> Bool { | ||
| let isValid = nickname.range(of: validNicknameRegex, options: .regularExpression) != nil | ||
| self.isValid.value = isValid | ||
| return isValid | ||
| } |
|
|
||
| func fetchDailyBoxOffice(key: String, targetDate: String, completion: @escaping (Result<[Movie], MovieError>) -> Void) { | ||
| provider.request(.dailyBoxOffice(key: key, targetDate: targetDate)) { result in | ||
| switch result { | ||
| case .success(let response): | ||
| do { |
There was a problem hiding this comment.
API 통신을 위한 API KEY를 외부에서 주입하시는군요!
저는 일반적으로 하나의 API KEY로 API를 다룬다고 생각해서 TargetType 안에서 가져오도록 구현하는데요.
혹시 외부 사용에서 API KEY가 없을 때 에러 처리로 대응하기 위한 구현일까요?
There was a problem hiding this comment.
동의합니다. 하나의 API KEY로 API를 다룬다고 생각해서 TargetType 안에서 가져오도록 구현하려고 했는데 마음처럼 잘 안된코드를 보고 계십니다!
Moya도 처음이고 API키를 숨긴것도 처음이라 생긴 코드인데 일단은 외부 주입에 따라 ErrorHandling으로 나눈 코드이며 이 부분은 좀더 공부해볼 생각입니다!
There was a problem hiding this comment.
저도 API KEY가 없을 때 에러 핸들링에 대해 고민해 보았는데요..
오히려 Target Type 안에서 구현했을 때는 API KEY가 없을 때에 오류를 해당 Target Type 생성 시에 오류를 throws하게 했어야 해서, 불편했던 것 같습니다.
오히려 지훈님의 코드처럼 외부에서 API KEY를 주입해 주는 방식이 더 괜찮아 보이네요.
다만 이렇게 된다면, API KEY를 모든 API 마다 주입해 주어야 한다는 또 단점이 있을 것 같기도 하고..
좀 더 클린 아키텍처 개념으로 다가가 본다면, Network를 제공하는 Repository에서 API KEY를 주입해야 하나? 하는 생각이 들게 되었습니다~
🔥 불꽃같은 코드리뷰 부탁드립니다!!
🔥 불꽃같은 코드리뷰 부탁드립니다!!
🔥 불꽃같은 코드리뷰 부탁드립니다!!
비즈니스 로직은 데이터를 어떤 규칙에 의해서 가공하여 사용하는 로직이라고 생각합니다. 이로인해 사람마다 생각이 다를수 있다 생각합니다.
UI를 나누고 필터 로직을 적용햇다고 해도 그 로직이 어떤 문제를 해결하거나 역할을 수행하기 위한 계산 가공로직이라면 그것도 비즈니스로직의 일종이라 생각합니다.
저는 이를 기준으로 삼아서 비즈니스로직을 분리해서 ViewModel을 분리했습니다!
📄 작업 내용
UI를 수정하지는 않았습니다. 로직적인 부분들만 수정했습니다.