[Step8]Feedback from JK

[jinios]

스토어앱 마지막단계 피드백 정리

[jinios]

네트워크 에러처리

데이터를 다운받아서 상위모듈을 세팅할때

수정 전

private class func set(url: URL, handler: @escaping((ItemHashData)->Void)) {
URLSession.shared.dataTask(with: url) { (data, response, error) in
if let response = response as? HTTPURLResponse, response.statusCode == 200, let data = data {
do {
let parsedData = try JSONDecoder().decode(ItemHashData.self, from: data)
handler(parsedData)
} catch {
print("Parse Error!!!")
}
} else {
print("데이터 에러!")
}
}.resume()
}

• JK: tryDownload() 나 set() 에서 Handler가 있는데 비정상적인 경우에는 print()만 하고 있네요. 상위 모듈로 handler를 통해서 전달해서 다음 상황에 대비할 수 있도록 처리해보세요.

수정 후

private class func set(url: URL, handler: @escaping((ItemHashData?)->Void)) {
        URLSession.shared.dataTask(with: url) { (data, response, error) in
            if let response = response as? HTTPURLResponse, response.statusCode == 200, let data = data {
                do {
                    let parsedData = try JSONDecoder().decode(ItemHashData.self, from: data)
                    handler(parsedData)
                } catch {
                    handler(nil)
                }
            } else {
                handler(nil)
            }
            }.resume()
    }

• JK: handler(nil) 을 호출하면 받는 쪽에서 바뀐게 있나요? 어디서 하는지 못 찾겠습니다. 어떤 동작을 해야할까요?
• 답변: 핸들러에 nil을 넘기면 DetailHash의 itemHashData 프로퍼티가 nil로 세팅됩니다. 테이블뷰 셀은 해당 값을 프로퍼티로 가지고만 있고, 상품상세페이지를 담당하는 뷰컨트롤러가 해당 셀에 있는 itemHashData값을 가지고 뷰를 구성하게되어있어서, 상위모듈에서 nil을 체크해서 상품 상세페이지 뷰를 보여줄지 말지 결정할 수 있습니다.

[jinios]

ViewController와 연결성 체크

ViewController에서 network status를 체크하거나 NetworkManager에서 noti를 받아서 처리하는 부분에 대한 코멘트

변경 전

// StoreViewController.swift
override func viewDidLoad() {
        ...
      // NetworkManager로부터 reachabilityChanged notification을 바로 받음
        NotificationCenter.default.addObserver(self, selector: #selector(dataReload), name: .reachabilityChanged, object: nil)
        NotificationCenter.default.addObserver(self, selector: #selector(setComplete(notification:)),name: .sectionSetComplete,object: nil)
        StoreItems.categories.forEach { (category) in
            self.storeItems.set(with: category)
        }
    }
// NetworkManager.shared.reachable을 가져와서 판단하여 처리
@objc func dataReload() {
        if NetworkManager.shared.reachable {
            StoreItems.categories.forEach { (category) in
                self.storeItems.set(with: category)
            }
        } else {
            print("RootView - dataReload. Not reachable")
        }
    }

// 마찬가지로 NetworkManager.shared.reachable를 판단하여 처리 
func tableView(_ tableView: UITableView, didSelectRowAt indexPath: IndexPath) {
        if NetworkManager.shared.reachable {
            self.runToast(indexPath: indexPath)

            if let nextVC = self.storyboard?.instantiateViewController(withIdentifier: "itemDetailView") as? ItemViewController {
                ...
                self.navigationController?.pushViewController(nextVC, animated: true)
            }
        } else {
            self.toUnreachableView()
        }
    }

• (코드 내에 주석으로 문제되는 내용 표시해놓음)

• 이 부분에 대한 전체적인 피드백: 테이블뷰의 DataSource를 담당하는 뷰 컨트롤러는 NetworkManager의 상태에 직접 접근해서 어떤 뷰를 보일지 판단하는게 아니라, StoreItems와 같은 모델객체에서만 NetworkManager의 status를 판별하고 뷰컨트롤러는 자신이 트랙킹하고있는 데이터모델(StoreItems)의 상태만 판별해서 뷰를 그리는 흐름으로 구현해야한다는 것. 아래는 자세한 피드백 내용

• JK:

  • dataReload 노티를 받았을 때 ViewController가 판단해서 데이터를 다른걸 채우는건가요? ViewController는 네트워크 상태를 알 필요없이, StoreItems에 (혹은 네트워크 관리하는 객체가 있다면...) 요청하면 이 동작을 StoreItems 에서 처리하는건 어떤가요?
  • DataSource가 네트워크 상태를 모르게 주어진 데이터로만 처리하도록 개선해보세요.
  • ViewController 가 .reachabilityChanged 상태를 옵저빙 해야 하는걸까요? 상태 변화를 감시할 게 아니라 그냥 데이터 모델이 바뀌었는지만 감시하면 되는거 아닐까요?

• 나: JK질문있습니다!! 네트워크 연결성을 확인하는 부분이 StoreItems나 DetailHash와 같은 모델로 제한하는 흐름으로 통일해야하는건가요? (DetailHash는 상품의 hash값으로 데이터를 받아와서 저장하는 모델입니다) 그러면 해당 모델에서 네트워크를 체크한 결과에따라 뷰컨트롤러가 동작할수있도록 알려주는 방식도 안되는건가요?_? (notification을 보낸다든지...하는 방식으로요)

• JK: ViewController가 네트워크 상태에 따라 모델을 다르게 접근하기 보다는 네트워크 상태를 관리하는 계층이 어디에서 동작할지 고민해보는 게 좋습니다. ViewController는 상태를 모르고, 모델에서 값이 올때마다 뷰에 반영하도록 단순하게 작성하는 게 좋습니다. VC --> Network Manager --> 상태확인후 Request 또는 저장된 데이터 처리 --> 모델 변경 --> 노티 --> VC 화면 변경 흐름을 상상해봤습니다.

변경 후

• 연결성이 바뀌는걸 체크하는 reachabilityChanged notification옵저버를 StoreItems객체로 옮김
• 네트워크 상태가 변할때마다 StoreItems객체가 리셋됨
• StoreItems는 새롭게 세팅될때마다 ViewController에 noti를 보내게되어있었으므로 해당부분 활용
• 뷰컨트롤러는 StoreItems에서 보내는 noti로 해당 모델의 세팅완료여부만 판단해서 테이블뷰를 reload하는 흐름으로 변경
• 상품 상세페이지를 표시할때 네트워크를 판별하는 부분에서도, 네트워크 상태를 판단하는게 아니라 상품 상세페이지를 구성하는 detailHash가 있는지 없는지를 구분해서 뷰 push

  
  func tableView(_ tableView: UITableView, didSelectRowAt indexPath: IndexPath) {
      if let nextVC = self.storyboard?.instantiateViewController(withIdentifier: "itemDetailView") as? ItemViewController {
          if let selectedCell = tableView.cellForRow(at: indexPath) as? StoreTableViewCell {
              if selectedCell.isHashDataEnable {
                  ...
                  self.navigationController?.pushViewController(nextVC, animated: true)
              } else {
                  self.toUnreachableView()
              }
          }
      }
  }

- **변경 후 흐름:**
  - reachabilityChanged notification: NeworkManager에서 네트워크 상태가 변경될때마다 post - Notification옵저버에 의해서 StoreItems 세팅되는 메소드 호출 - StoreItems가 세팅이 완료되면 ViewController에서 테이블뷰 reload