[iOS] PR review 반영

[Lia316]

수정 사항

• [x] 코드형식 (공백)
• [x] 네이밍
• [x] cell identifier
• [x] enum vs boolean
• [x] ViewController Factory
• [x] 디미터 법칙
• [x] NetworkManager 의존성
• [ ] MVVM
  • [x] ViewController binding
  • [x] GameCell binding
  • [x] DiffableDataSource → DataSource

5월 12일 수요일 할 일

• [x] 롤로: static 질문 정리해서 올려주시기
• [x] 리아: MVVM 패턴대로 코드 정리해서 내부 PR 올리기
• [x] 롤로: 내부 PR 리뷰
• [x] 리아: Jason review 댓글 소통

[Lia316]

작성된 스타일을 토대로 기준을 정리했습니다

🔸 class & extension

: 위 아래 공백 한 칸씩, 함수 간 공백 한 칸씩

🔸 struct

: 위 아래 공백 없음, 프로퍼티 간 공백 없음, 새로운 struct나 enum 타입 쓸 경우만 공백 있음.

🔸 enum & protocol

: 공백 없음

🔸 function

: 위 아래 공백 없음

봐주셨으면 하는 타입

struct Turn: Decodable {
    var inning: [Int]
    var home: Team
    var away: Team
    var ballCounts: [Int]
    var baseInfo: [Bool]
    var pitches: [Pitch]

    enum CodingKeys: String, CodingKey {
        case inning
        case home
        case away
        case ballCounts
        case baseInfo
        case pitches = "list"
    }

    struct Team: Decodable {
        var name: String
        var score: Int
        var role: String
        var player: Player

        enum CodingKeys: String, CodingKey {
            case name = "team"
            case score
            case role
            case player
        }
    }
}

@eeeesong 이 부분은 원래 의도가 Team 타입을 바로 밑에 둬서 알아보기 편하게 하신 것 같아 보였습니다. 공백을 없애려다 보니까 프로퍼티가 두 그룹으로 나눠지는 느낌이 있어서 한 곳에 모았습니다. 혹시 이 부분 원래 의도대로 바꾸고 싶으시다면 말씀해주세욥!!

[eeeesong]

@Lia316 아아 리아!! 합친 것 좋습니다 다만 저희 논의 이후 만든 DTO가 이거랑 꽤나 다르게 생겨서... 이후에 모두 수정해버렸어요ㅠㅠ 그래서 GameDTO 부분 커밋을 합치면 Turn은 아예 날아갈 것 같네요ㅠㅠ

[Lia316]

의존성 주입

class NetworkManager {

    private let session: URLSession

    init(session: URLSession = .shared) {
      self.session = session
    }

    ...
}

의존성 역전

protocol NetworkManageable {
    func get<T: Decodable>(type: T.Type, url: URL) -> AnyPublisher<T, Error>
    func post<T: Codable>(url: URL, data: T) -> AnyPublisher<Void, Error>
}

extension NetworkManager: NetworkManageable { ... }

하고, static 메소드를 모두 인스턴스화했습니다.

[Lia316]

고민

static 및 공유 인스턴스의 효과적인 사용법 위의 url 을 보면,

공유 인스턴스를 사용할 때에도, 다른 인스턴스를 사용할 수 있는 경우를 대비해 예비 방안(built-in escape hatch)을 만들어 두세요 라는 말이 나옵니다.

제가 초기에 static 메소드를 설정한 이유는, get과 post가 독립적이고, NetworkManager 클래스에 프로퍼티도 없기 때문에 굳이 객체를 생성할 필요가 없다고 판단해서였습니다.

그런데 URLSession 중에서 다른 인스턴스를 쓰고 싶은 경우를 대비해서 생성자를 만들면.. 어쩔 수 없이 static 도 포기해야합니다. 객체 생성에 비용을 들이는 것 vs URLSession 의존성 분리 측면에서 후자가 더 중요한 걸까요??

아! 그리고 말씀하신 테스트 케이스의 이유도 있네욤

[Lia316]

DiffableDataSource 논의

MVVM 패턴은 뷰모델에 모델을 다루는 원칙을 철저히 지키는 것으로 준수하고, DiffableDataSource는 살리기로 논의함

[Lia316]

ViewController 생성 Factory

Factory 클래스

class ControllerFactory {
    static func instantiate(viewController: Instantiatable.Type) -> UIViewController {
        return viewController.instantiate()
    }
}

프로토콜

protocol Instantiatable {
    static func instantiate() -> UIViewController
}

메소드 정의

extension LoginViewController: Instantiatable {

    static func instantiate() -> UIViewController {
        let myViewController = UIStoryboard(name: "Main", bundle: nil).instantiateViewController(withIdentifier: "LoginViewController") as? LoginViewController

        return myViewController ?? LoginViewController()
    }

}

실제 구현부

let rootViewController = ControllerFactory.instantiate(viewController: LoginViewController.self) as! LoginViewController

[Lia316]

reuse identifier protocol

코드 구현부

protocol IdentifierReusable {
    static var reuseIdentifier: String { get }
}

extension IdentifierReusable {
    static var reuseIdentifier: String {
        return String(describing: self)
    }
}

instantiateViewController(withIdentifier: self.reuseIdentifier)

고민한 부분

    enum ViewID {
        static let storyboard = "GamePlay"
        static let segue = "selectPitcher"
    }

Jason 이 말씀하신 것처럼 저도 enum 을 사용한 부분이 훨씬 명시적이라 다 적용하고 싶었는데 🥲 나머지 뷰컨트롤러들은 변수를 하나만 갖게 되어서 GamePlayViewController에만 ViewID를 남겨놨습니다. 코드 일관성이 살짝 떨어질까 걱정은 되지만, ViewID 타입이 있는 게 좋을 것 같았습니다!

적용한 파일

• LoginViewController
• SelectionViewController
• GamePlayViewController
• PitchListTableViewCell
• GameCell

참고

1. 영어 자료 2. 한글 블로그

[Lia316]

enum vs Bool

회의 후 일단 enum 으로 구현하기로 함!

형태

enum TeamSide: String, Codable {
    case home
    case away
}

기능은 차이가 없게 코드만 교체함

[eeeesong]

enum TeamSide: String, Codable {
    case home
    case away
}

기능은 차이가 없게 코드만 교체함

넵! 이후 DTO -> GameManager 매핑 시 enum에 맞게 구현하겠습니다!

[eeeesong]

Static?

URLSession.shared에 관하여...

리뷰어께서 주신 글 다시 읽어보고 특히 URLSession에 관해서 다시 생각을 해보았습니다!

Static이어도 괜찮지 않을까? 생각했던 이유가 shared instance인 URLSession.shared를 쓰니까 그랬던 건데, 꼭 shared를 쓰지 않더라도 커스텀할 수 있는 부분이 많더군요! 특히나 테스트를 작성하게 된다면 proxy를 만들어서 가짜로 테스트도 할 수 있는 것 같아서 그렇다면 외부에서 URLSession 객체를 주입해주는 방식이 확실히 더 유연할 것 같다는 생각이 들었습니다.

하지만 우리는 사실 shared 말고 다른 URLSession을 쓸 일이 없으므로... medium 글에 나와있는 것처럼 디폴트 값을 설정한 이니셜라이저를 쓰는 것이 최선이 아닐까합니다!

init(urlSession: URLSession = URLSession.shared) {
    self.urlSession = urlSession
  }

그래서... 이 부분에 대해서는 저는 전보다 확실히 명확해진 것 같아서 질문까진 안해도 될 것 같네요 :)

static vs. instance

다만 이 포인트 말고 궁금한 부분이 있다면 네트워크 메소드 자체를 static하게 만드는 것보단 instance로 두는 게 나은 이유가 아래의 두가지 인 것으로 이해를 했는데,

1. 객체 지향 프로그래밍 원칙에 더 맞기 때문에
2. 굳이 static으로 둘만큼 생성 비용이 높지 않아서

이 이유가 맞는지 확답을 얻으면 좋을 것 같긴 합니다! @Lia316

[Lia316]

꼭 shared를 쓰지 않더라도 커스텀할 수 있는 부분이 많더군요! 하지만 우리는 사실 shared 말고 다른 URLSession을 쓸 일이 없으므로...

저도 이것 때문에 우선 디폴트가 있는 생성자를 만들어서 코드를 고쳐봤었습니다! 다만 진짜 우리는 shared 말고 다른 URLSession을 쓸 일이 없을 것 같아서 ㅋㅋ 여전히 '꼭 static을 포기해야하나?! '하는 생각이 들었었는데,

말씀해주신대로

1. 객체 지향 프로그래밍 원칙에 더 맞기 때문에
2. 굳이 static으로 둘만큼 생성 비용이 높지 않아서

이 부분을 질문 드려서 명확하게 되면 저도 의문이 좀 풀릴 것 같습니다 :)

😀 롤로가 깔끔하게 정리해주셔서 최대한 느낌 살려서 질문 올려보도록 하겠습니다! @eeeesong

[Lia316]

MVVM 패턴대로 수정

cell logic 삭제

GameCell.swift

func updateUI(with viewModel: SelectViewModel) {
guard let model = viewModel.cellInfo else { return }

    self.gameIdLabel.text = model.gameID
    self.awayTeamButton.setTitle(model.away, for: .normal)
    self.homeTeamButton.setTitle(model.home, for: .normal)

    self.awayTeamButton.isEnabled = model.isAwayEnable
    self.homeTeamButton.isEnabled = model.isHomeEnable
}

cell 에 비교하거나 형변환하던 코드를 다 삭제함
오직 뷰모델에 있는 모델 값으로 화면 띄우게 변경

### cell 내부 버튼 touch 시 정보 전달 로직
> GameCell.swift
```swift
    @Published var userTeam: String!

    @IBAction func awayButtonTouched(_ sender: UIButton) {
        self.userTeam = sender.currentTitle
        self.awayTeamButton.isEnabled = false
    }

SelectionViewController.swift

private func bindSelection(with cell: GameCell) {
cell.$userTeam
.receive(on: DispatchQueue.main)
.sink { [weak self] userTeam in
guard let userTeam = userTeam else { return }
self?.viewModel.selected(team: userTeam)
}
.store(in: &cancelBag)
}

    private func configureDataSource() {
        self.dataSource = UITableViewDiffableDataSource.init(tableView: self.gameListTableView) { (tableView, indexPath, game) -> UITableViewCell in

            let cell = self.gameListTableView.dequeueReusableCell(withIdentifier: GameCell.reuseIdentifier) as! GameCell

            self.viewModel.setCellInfo(with: game)
            self.viewModel.delegate = self

            cell.updateUI(with: self.viewModel)
            self.bindSelection(with: cell)

            return cell
        }
    }

SelectViewModel.swift

func selected(team: String) {
self.gameInfo.team = team

    self.postSelection(with: self.gameInfo)
    self.delegate.didPressButton(with: self.gameInfo)
}

### Network 처리 변경
NotificationCenter 모두 제거하고 ViewController 에서 bind 하도록 변경
(롤로 코드와 똑같다고 보시면 됩니다!)

### 디미터 법칙 수정
> LoginViewController.swift
```swift
nextVC.setUserID(with: IDTextField.text ?? "")

SelectionViewController.swift

func setUserID(with userID: String) {
self.viewModel.setUserID(with: userID)
}

[Lia316]

@eeeesong 이제 내부 피알 올리겠습니다! 😂 리뷰 기다리는 동안 Jason 리뷰 댓글 작성하고 있겠습니다!!

[Lia316]

수정 및 소통 완료