BID-91 홈화면 서비스레이어 분리 및 리팩토링#52
Conversation
| if let item = response.data?.getItem{ | ||
| observer(.success(item)) | ||
| return | ||
| } |
| if let item = response.data?.getItemList{ | ||
| observer(.success(item)) | ||
| return | ||
| } |
There was a problem hiding this comment.
여기도 item이 invalid한 경우에 대한 케이스에 대한 return 이 없네요
observer(.failure(...)) 같은 처리가 필요해 보입니다
| if let item = response.data?.getEndingSoonItems{ | ||
| observer(.success(item)) | ||
| return | ||
| } |
There was a problem hiding this comment.
여기도 item이 invalid한 경우에 대한 케이스에 대한 return 이 없네요
observer(.failure(...)) 같은 처리가 필요해 보입니다
| self.itemRepo = itemRepo | ||
| } | ||
|
|
||
| func getItem() -> Single<GetItemQuery.Data.GetItem> { |
There was a problem hiding this comment.
가져올 item의 ID를 input parameter로 넣어야 하지 않을까요?
There was a problem hiding this comment.
아이템 상세화면 작업할 때 추가하겠습니다.
| } | ||
|
|
||
| func getItem() -> Single<GetItemQuery.Data.GetItem> { | ||
| return self.itemRepo.getItem(id: 1) |
There was a problem hiding this comment.
id:1 이부분은 테스트를 위해서 임시로 hard coding된거 같은데 이건 테스트 하실때만....
| return self.itemRepo.getItem(id: 1) | ||
| } | ||
|
|
||
| func getEndingSoonItems() -> Single<[GetEndingSoonItemsQuery.Data.GetEndingSoonItem?]> { |
There was a problem hiding this comment.
GetEndingSoonItemsQuery.Data.GetEndingSoonItem? 이건 굳이 optional로 전달해야할 이유가 있을까요?
There was a problem hiding this comment.
Item이 없는 경우를 생각했던건데 빈배열로 전달이 가능하므로 Optional일 필요가 없겠군요?
수정하겠습니다
|
|
||
| class EndingSoonReactor : Reactor { | ||
|
|
||
| var itemList : [Item] = [] |
There was a problem hiding this comment.
여기에 itemList 를 가지고 있어야 하는 이유가 있을까요?
| /* | ||
| Data.GetItemList -> [Item] 변환 | ||
| */ | ||
| func toEndingSoonItemList(data : GetItemListQuery.Data.GetItemList) -> [Item]{ |
There was a problem hiding this comment.
이런 것들은 service 혹은 adapter class를 만들어서 그쪽에 들어가야 할것 같네요
| .asObservable() | ||
| .observe(on: MainScheduler.instance) | ||
| .flatMap{// GetItemListQuery.Data.GetItemList | ||
| let itemList = self.toEndingSoonItemList(data: $0) |
There was a problem hiding this comment.
toEndingSoonItemList() method의 기능이 service에 들어가야 할것 같네요
보여주기 위한 모양을 만드는 역활이라면 adapter class를 만들어서 처리해야 할것 같네요
| .asObservable() | ||
| .observe(on: MainScheduler.instance) | ||
| .flatMap{// GetItemListQuery.Data.GetItemList | ||
| var itemList = self.toEndingSoonItemList(data: $0) |
There was a problem hiding this comment.
toEndingSoonItemList() method의 기능이 service에 들어가야 할것 같네요
보여주기 위한 모양을 만드는 역활이라면 adapter class를 만들어서 처리해야 할것 같네요
| user: userInfo | ||
| ) | ||
|
|
||
| if tempItem.status != 3 && tempItem.status != 4{ |
There was a problem hiding this comment.
3, 4 요런거는 enum이나 define되어 있는 걸 사용해서 어떤상태를 의미하는지 알수 있도록 하는게 좋을것 같네요.
| .flatMap{ data in | ||
| let data = try JSONSerialization.data(withJSONObject: data.jsonObject, options: .sortedKeys) | ||
| let decode = try JSONDecoder().decode(Item.self, from: data) | ||
| print(decode) |
There was a problem hiding this comment.
요건 디버깅을 위한거 같은데 임시로 사용하는게 아니라면 형식을 조금 갖추는게 좋아요. 예를 들어 클래스,함수,라인 같은것들요.
참고로, 제가 생성해둔 logger를 사용하시면 따로 만들필요없어요
| let decode = try JSONDecoder().decode(Item.self, from: data) | ||
| print(decode) | ||
| let inputSection = configSections(item: decode, bidList: nil) | ||
| return Single<Mutation>.just(Mutation.updateItemData(inputSection)) |
There was a problem hiding this comment.
여기서 Mutation.updateItemData(inputSection) 이걸 호출 하는 것 보다는
Mutation.updateItemData(inputSection) 내에서 동작하는 것들을 따로 method 로 만들고 만든 method를 여기와 다른 사용하는 곳에서 호출하는게 좋죠
그리고 , 기본적으로 mutation은 내부에서 rx를 사용하기 때문에(만약 rx를 사용하지 않는 case가 있다면 mutation에 있으면 안되겠죠?) 이런식으로 호출하는 것은 피해야 하고 chaining을 하는게 좋아요
| case .updateBidList(let bidList): | ||
| state.sections = configSections(item: self.currentState.item, bidList: bidList) | ||
| if let item = self.currentState.item { | ||
| state.sections = configSections(item: item , bidList: bidList) |
There was a problem hiding this comment.
직접 assign하는게 아니라 rx형태로 사용되는게 좋을거 같네요..
|
|
||
| if reactor.currentState.item.status < 2{ | ||
| self.setDirectPopup(item: reactor.currentState.item) | ||
| if reactor.currentState.item?.status ?? 0 < 2{ |
There was a problem hiding this comment.
reactor의 상태를 여기서 직접 보는게 아니라 reactor에 Action을 보내고 reactor에서 service를 통해서 해당 항목이 즉시 구매가 가능한지 확인하고 -> state를 업데이트 해서 -> viewcontroller에서 결과에 따른 ui를 보여주는 형태의 rx로 구현 하는게 일관성있는 구조를 유지 하는것 아닐까요?
2 participants
아직 적용이 안 된 cursor기반 페이징 기능을 구현해보고
다음은 ItemDetailScene부분을 서비스 레이어 분리를 해볼 생각입니다.