[team-01][쑤, Isaac] Cache, detailView 구현 #55
Conversation
refactor : RequestBody 형태로 데이터를 전달받아라.
|
규모가 급격하게 커지면서 코드를 제어하는데 어려움을 경험하셨군요. |
|
안녕하세요 강운! |
|
|
||
| detailViewModel.imageFetchHandler = { | ||
| DispatchQueue.main.async { | ||
| self.setTitle() |
There was a problem hiding this comment.
해당 클로저를 봤을 때 어떤 데이터가 오고 어떻게 처리하겠다가 눈으로 확인할 수 없습니다.
메소드를 작성하실 때
Input -> Output 그리고 해당 Output을 적용,
이를 추출, 연산, 적용 세가지로 나누게 되면, 나중에 코드에서 버그가 났을 때 의심할 수 있는 코드를 줄일 수 있는 아주 효과적인 방법입니다.
| Toast(text: error).show() | ||
| } | ||
|
|
||
| NotificationCenter.default.addObserver(self, selector: #selector(increaseQuantity), name: .increaseQuntity, object: nil) |
There was a problem hiding this comment.
코드를 보게되면 Delegate, Notification, closure 모두 사용하시는데, 각자 다른 이유가 있을까요?
| } | ||
|
|
||
| @objc func increaseQuantity() { | ||
| detailViewModel.quantity += 1 |
There was a problem hiding this comment.
매번 햇갈리시는 거 같습니다.
-
뷰는 뷰모델을 받아서 뷰를 꾸미는 역할만합니다.
현재 viewController를 확인해보면 viewModel에서 해야할 데이터 가공에 직접적으로 연관되어 있습니다. -
제가 강조했던 추상화가 전혀 이루어지고 있지 않습니다.
뷰는 뷰모델에게 "action" 즉 "메세지"만 보내고 그 메세지를 해석하고 일을 처리하는 건 viewModel이어야 합니다.
| private func setThumdnailImage() { | ||
| let currentDetailItem = self.detailViewModel.currentDetail | ||
| for index in 0..<(currentDetailItem.thumbImagesData?.count ?? 0) + 1{ | ||
| let imageView = UIImageView(frame: CGRect(x: thumbnailScrollView.frame.width * CGFloat(index), |
There was a problem hiding this comment.
뷰를 프레임값을 설정해서 잡는거 말고 다른 방법은 없을까요?
해당 코드는 사실 1번 이미지는 ((0,0), (100, 100), 2번 이미지((100,0), (100,100)) 이렇게 하드 코딩으로 뷰를 잡아주고 계신겁니다.
There was a problem hiding this comment.
앗 그렇군요! 하드코딩의 범주를 좀 더 넓혀서 봐야겠습니다 stackView를 사용해서 thumbnail 부분을 수정해봤습니다!
| import Foundation | ||
|
|
||
| struct DetailItem: Codable { | ||
| var topImageURL: String |
There was a problem hiding this comment.
해당 프로퍼티가 꼭 필요한가요?
앱을 사용하실 때 이상한 점을 못 느끼셨나요?
| func cachingImage(named: String, imageData: Data) | ||
| } | ||
|
|
||
| class ImageCacheCenter: ImageCacheable { |
There was a problem hiding this comment.
ImageCacheCenter와 SidedishNetworkCenter 관계가 명확하게 말씀하실 수 있나요?
후자는 전자를 참조해서 어떤일을 처리하고 전자는 후자가 하는 일도 하고 있는 것처럼 보입니다.
각 객체에 역할을 확실하게 분리하실 필요가 있습니다.
| @@ -27,13 +34,82 @@ class SidedishNetworkCenter: Networkable { | |||
| } | |||
| } | |||
|
|
|||
| func fetchDetail(url: String, completion: @escaping (Result<DetailItem, AFError>) -> ()) { | |||
There was a problem hiding this comment.
비슷한 일을 하는 메소드를 줄일 수 있는 방법은 없을까요?
해당 메소드를 사용하는 곳에서 타입을 지정해서 받아올수 있게 변경하실 수 있을까요??
제네릭에 대해서 아직 서투르시다면 넘어가셔도 좋습니다.
There was a problem hiding this comment.
줄여볼 수 있지 않을까 생각했었는데 '메소드를 사용하는 곳'이 키포인트였군요! 감사합니다 :)
하지만 수정 후에 보니 type -> T, T -> type 간 guard 문이 자주 사용돼서 좀 더 깔끔한 방법이 있는지 좀 더 고민해보겠습니다!
| } | ||
|
|
||
| private func fetchDetailImgae(detail: Detail, completion: @escaping (Detail) -> ()) { | ||
| let originalDetail = detail |
There was a problem hiding this comment.
하나의 메소드는 한가지의 작업을 수행하는게 유지보수하는 입장에선 편하실겁니다.
오류가 발생해도 해당 영역이 줄어들면 문제를 찾기 편해지실 겁니다.
| } | ||
|
|
||
| @IBAction func sendOrder(_ sender: Any) { | ||
|
|
There was a problem hiding this comment.
바쁘셔서 놓치셨을테지만,
구현하지 않는 코드는 작성하지 않는게 좋습니다.
| } | ||
|
|
||
| private func loadItems() { | ||
| Category.allCases.forEach { (category) in |
There was a problem hiding this comment.
해당 부분도 뷰에서 "Category에 있는 모든 데이터를 받을래"라고 말하는게 아니라
뷰모델에게 "나 데이터를 받고 싶어" 메세지를 전달하는 방식이여야합니다.
작업 목록
main,soup,side에 따라 section 별로 구분학습 키워드
고민과 해결(질문거리), 해결하지 못한 점
Data(contentsOf: )의 오용을 막기 위해 URLSession을 사용하게 되면서 cache 처리 등으로 Repository 부분이 굉장히 지저분해진 것 같습니다. 각각의 image 부분을 받아오게 되면서 행동들을 처리하고 있는데 메소드가 비대해진 것 같습니다. 뭔가 깔끔하게 해보고 싶었지만 생각이 나질 않아 고민이 됐지만 아직까지 고민중에 있습니다.xib를 사용해서 상품 정보에 대한 뷰를 분리해서 작업했는데, 런타임에 constraint error log가 출력되는 문제를 해결하지 못했습니다.
(원인을 조금 더 살펴봐야 할 것 같습니다..)
@GangWoon
이번 PR에서는 상세 화면 구성과, 이미지 캐싱 처리 작업을 주로 구현하였습니다. 상세 화면을 scroll view를 이용해서 구현하는데 다소 시간이 걸려 PR이 늦어진 점 양해부탁드립니다.😥