박스오피스II [STEP 2] Prism, Gray #135
Conversation
Co-authored-by: yawoong2 <[email protected]>
Co-authored-by: yawoong2 <[email protected]>
Co-authored-by: yawoong2 <[email protected]>
There was a problem hiding this comment.
- ContentView를 활용한 ListContentView와 GridContentView 서브클래싱
위의 고민했던 점에서 적었듯 ListContentView와 GridContentView의 공통되는 부분을 모아놓은 ContentView 클래스를 정의하고, 두 contentView가 이를 서브클래싱해서 사용하도록 했습니다. 다만 이럴 경우 각 view를 한눈에 파악하기 힘들고(SubClass에 없는 컴포넌트들을 SuperClass에서 찾아 확인해야 함), ContentView(SuperClass)의 경우 반드시 서브클래싱한 후 사용해야 하나 직접 사용하는 경우가 발생할 수 있다는 문제가 있습니다.
중복되는 코드가 있더라도 개별적인 ListContentView와 GridContentView를 정의하는게 좋은지, 중복 코드는 없지만 가독성과 안전성에 있어 손해를 보는 subclassing을 사용하는게 좋은지 조언을 구하고 싶습니다.
이건 상황에 따라, 개발자에 따라, 회사에 따라 다른 내용이긴 한데요
제 생각에는 재사용이 되더라도 계속해서 두 피쳐가 방향성이 달라질 경우,
재사용하는게 오히려 더 복잡해질수도 있는 상황이 많았어요!
따라서 프로토콜과 extension으로 default 구현을 활용하던다,
혹은 상속할 객체를 최소한으로 구현하는게 경험상 유지보수하기 더 좋았던거 같아요!
따라서
- Cell은 두가지 셀 타입을 만들어서 register해주고
- 각각의 셀을 가지는 두가지 콜렉션뷰를 만들던가
- 별개의 view를 만들고 viewController의 행동만 공유하던가
- 극단적으로 뷰컨까지 별개로 들고있는것도 생각해볼 수 있을거 같아요
- NetworkService의 injection?
현재 AppDelegate에서 NetworkService의 인스턴스를 생성 후 DailyBoxOfficeViewController로 주입해주고 있는데, 이 인스턴스를 DailyBoxOfficeDetailViewController에서도 사용해야 합니다. 주입받은 인스턴스를 그대로 detailView로 다시 주입하는 것이 좋은지, 아니면 Singleton 패턴을 활용하는 것이 좋은지 조언을 구하고 싶습니다.
같은 네트워크를 사용해야한다면 다시 주입해주면 될 것 같고,
다른 네트워크를 사용해야한다면 새 객체를 생성해서 주입해주면 될 것 같아요!
Singleton은 전역적으로 관리할 "상태값"이 있을 때 유용한데요,
사실 이 경우에 사용해도 무방하긴 해요
의존성 주입이 많아지고 귀찮아지다보면
- factory pattern
- DIContainer
- Property Wrapper를 활용한 주입
등등을 키워드로 관련 자료들 찾아보시면 좋을 듯 합니다!
| override func viewWillAppear(_ animated: Bool) { | ||
| navigationController?.isToolbarHidden = true | ||
| } | ||
|
|
||
| override func viewWillDisappear(_ animated: Bool) { | ||
| navigationController?.isToolbarHidden = false | ||
| } |
| } | ||
| } | ||
| case .failure(let error): | ||
| DispatchQueue.main.async { |
There was a problem hiding this comment.
DispatchQueue를 감싸줄때도 웬만하면 [weak self] 잡아주시는게 좋아요!
delay deallocation이 일어날수도 있어서요!
| fetchPosterURL(of: movieName) { result in | ||
| switch result { | ||
| case .success(let url): | ||
| self.fetchPosterImage(from: url) { result in |
There was a problem hiding this comment.
여기서 strong self가 넘어가면서 62번 self.present에서 메모리릭이 발생할수도 있을거 같네요!
| func collectionView(_ collectionView: UICollectionView, didSelectItemAt indexPath: IndexPath) { | ||
| let dailyBoxOfficeDetailViewController = DailyBoxOfficeDetailViewController(selectedMovieCode: boxOffices[indexPath.row].movieCode, selectedMovieName: boxOffices[indexPath.row].title) |
There was a problem hiding this comment.
boxOffices의 subscript에 접근할 떄에는 safe subscript를 만들어서 안전하게 조회해주세요!
https://minsone.github.io/programming/check-index-of-array-in-swift
| // BoxOffice | ||
| // | ||
| // Created by kjs on 13/01/23. | ||
| // | ||
|
|
||
| import UIKit | ||
|
|
||
| final class DailyBoxOfficeListViewController: UIViewController { | ||
| enum ScreenMode { |
There was a problem hiding this comment.
이 enum은 앱 전반적으로 사용하기 보다는 특정화면 or 피쳐에서만 사용하는 이넘인거 같아요!
네임스페이스를 잡아주거나, 뷰컨 등 nested하게 존재하면 좋을거 같아요
| if let queryParameters { | ||
| components.queryItems = queryParameters.map { | ||
| URLQueryItem(name: $0.key, value: $0.value) | ||
| } |
There was a problem hiding this comment.
요러한 친구들이 생기고 수정될 수 있다는 점에서, 전에 리뷰드렸던것처럼 객체를 분리해서(Interceptor)등을 활용하여 역할분리를 하면 좋습니다 ㅎㅎ
참고만 해주세요
| return NetworkingFailAlert.makeAlert(error: error) | ||
| case let error as DecodingError: | ||
| return JSONDecodingFailAlert().makeAlert(error: error) | ||
| return JSONDecodingFailAlert.makeAlert(error: error) | ||
| default: | ||
| return UnknownErrorAlert().makeAlert(error: error) | ||
| return UnknownErrorAlert.makeAlert(error: error) |
There was a problem hiding this comment.
요렇게 구현하는 방법도 있고,
각각의 에러마다 title, subtitle, message 등을 정의하고,
func error(title: String, message: String) { ... } 요런 함수를 만들수도 있을거 같아요
안녕하세요. @havilog!😃
Prism, Gray 프레이🙏조 입니다.
박스오피스II 프로젝트의 STEP 2 가 완료되어 PR 요청드립니다!
시간이 부족해서 진행하지 못한 박스오피스I Step4를 함께 진행하느라 좀 오래걸렸습니다 💦
📱 실행화면
Activity Indicator 표시
(List Layout)
(Grid Layout)
🤔 고민했던 점
- Cell Configuration과 ContentView에 대한 고민(01354f1)
처음에 Grid Layout에 대한 구현을 할 때, List와 Grid layout 모드에 대한 Cell, Configuration, ContentView를 각각 두개씩 정의해주었습니다. ListCell과 GridCell, 그리고 List Configuration과 Grid Configuration의 구조가 거의 동일하기 때문에, 타입을 하나만 두고 Cell의 updateConfiguration 메서드가 screenMode를 파라미터로 넘겨주고, configuration에서는 받은 파라미터에 따라 다른 contentView를 넘겨주도록 해 중복되는 코드를 줄일 수 있었습니다.
ContentView의 경우에는 화면 모드(List,Grid)에 따라
DailyBoxOfficeListContentView,DailyBoxOfficeGridContrentView클래스로 나누어 ContentView를 보여줄 수 있도록 구현했습니다. 두 ContentView에서 중복되는 레이블 선언부와updateConfiguration()메서드가 있어DailyBoxOfficeContentView생성하여, 공통된 코드를DailyBoxOfficeContentView에 작성하고 ListContentView와 GridContentView가 이 클래스를 상속하도록 하여 중복된 코드를 줄일 수 있도록 했습니다.랭킹 등락을 컬러가 있는 삼각형이 포함된 문자열로 변환하는 코드는 protocol extension의 default implementation을 활용해 작성했습니다.
- Cell Registration(fed5c67)
ListCell과 GridCell을 따로 사용했을 때 cell을 등록하고 등록한 cell을 dataSource에 활용하기 위해
UICollectionView.CellRegistration<DailyBoxOfficeListCell, BoxOffice>,UICollectionView.CellRegistration<DailyBoxOfficeGridCell, BoxOffice>와 같이 타입이 달라 cell을 등록하고 dataSource를 지정하는 코드가 중복되었지만,DailyBoxOfficeCell만을 활용하도록 변경 후 코드의 중복을 제거할 수 있었습니다.- Layout 생성 코드의 분리(2637eea)
화면 모드에 따라 적절한 layout을 반환하는
getLayout(of:)메서드가 ViewController에 존재했지만, ViewController의 비대화를 피하기 위해 따로 분리하는게 좋다고 판단했습니다. 화면 모드에 따른 layout을 관리하는 LayoutManager 타입을 정의하고 사용해ViewController의 코드를 줄일 수 있었습니다.🙏 조언을 얻고 싶었던 점
- ContentView를 활용한 ListContentView와 GridContentView 서브클래싱
위의 고민했던 점에서 적었듯 ListContentView와 GridContentView의 공통되는 부분을 모아놓은 ContentView 클래스를 정의하고, 두 contentView가 이를 서브클래싱해서 사용하도록 했습니다. 다만 이럴 경우 각 view를 한눈에 파악하기 힘들고(SubClass에 없는 컴포넌트들을 SuperClass에서 찾아 확인해야 함), ContentView(SuperClass)의 경우 반드시 서브클래싱한 후 사용해야 하나 직접 사용하는 경우가 발생할 수 있다는 문제가 있습니다.
중복되는 코드가 있더라도 개별적인 ListContentView와 GridContentView를 정의하는게 좋은지, 중복 코드는 없지만 가독성과 안전성에 있어 손해를 보는 subclassing을 사용하는게 좋은지 조언을 구하고 싶습니다.
- NetworkService의 injection?
현재 AppDelegate에서 NetworkService의 인스턴스를 생성 후
DailyBoxOfficeViewController로 주입해주고 있는데, 이 인스턴스를DailyBoxOfficeDetailViewController에서도 사용해야 합니다. 주입받은 인스턴스를 그대로 detailView로 다시 주입하는 것이 좋은지, 아니면 Singleton 패턴을 활용하는 것이 좋은지 조언을 구하고 싶습니다.