은행 창구 매니저 [STEP4] 파프리, 마이노

[Mino777]

@Monsteel
안녕하세요 토니!!!
STEP4 구현 완료하여 PR 요청 드립니다!
2주동안 정말 좋은 리뷰들 주셔서 감사했습니다😆
앞으로도 궁금한점이 생기면 토니에게 달려가도록 하겠습니다!
감사합니다!!🙇‍♂️

---

구현내용

• 은행원이 일을 진행함에 따라 대기중인 고객과 업무중인 고객을 View에 표시
• 고객 추가와 초기화 진행 시 View에 해당사항 반영
• 코드만으로 UI 구현
• CompositionRoot
• Timer
• Operation Queue

---

실행화면

---

고민한 점 및 질문

1. Xcode 프로젝트 관리 구조

• 기존에 구현한 ConsoleApp 프로젝트의 파일들이 UIApp을 구현하는데에도 필요했습니다.
• ConsoleApp에서 reference로 LinkedList.swift , Queue.swift 를 가져오고 나머지 파일은 구조의 변경이 있을 거라 예상되어 UIApp 프로젝트에서 직접 생성하였습니다.
• 이 방법에 대해 토니는 어떻게 생각하시나요?

2. Timer 실행 시 딜레이

• Timer의 timeInterval을 0.001초로 진행하는 경우 실제 시간과 거의 2 ~ 3초 차이가 나는 현상이 발생했습니다.
• 우선 해당 값을 0.003초로 진행하여 현재는 0.2~3초 차이가 나도록 임시방편으로 두었습니다.
• 해당 부분은 크게 고민하지 않고 넘어가도 되는 부분일까요? 아니면 저희가 놓친 부분이 있는지 궁금합니다!

3. OperationQueue

• 초기화 시에 대기중이던 작업들을 취소해주는 요구사항이 있었습니다.
• 하지만 DispatchQueue의 경우엔 현재 대기중인 작업들을 취소할 수 있는 방법이 없었습니다.
• 따라서 OperationQueue로 변경한 후 cancelAllOperations() 메서드를 사용해 해당 요구사항을 구현할 수 있었습니다.

4. ScrollView Scroll시에 Timer가 멈추는 현상

• ScrollView를 Scroll하는 경우 MainRunLoop에서 타이머 레이블을 업데이트하는 부분과 충돌이 일어난다고 생각했습니다.

RunLoop.current.add(timer ?? Timer(), forMode: .common)

• 그래서 Timer의 RunLoop의 우선순위를 common으로 높여서 해당 부분을 해결할 수 있었습니다.

5. 코드로 UI 구현 시 발생한 문제 해결

because they have no common ancestor. Does the constraint or its anchors reference items in different view hierarchies? That's illegal.

• 위와 같은 에러문구가 나타난 문제를 addSubView/addArragedSubView 를 한 뒤, constraint를 정해줌으로써 해결했습니다.

6. NotificationCenter 활용

func work(client: Client) {
        NotificationCenter.default.post(name: .didAssignClientToBankClerk, object: client)
        Thread.sleep(forTimeInterval: self.bankService.requiredTime)
        NotificationCenter.default.post(name: .didFinishWork, object: client)
    }

• 은행원이 고객 일처리를 시작하는 순간과 일처리를 마무리하는 순간 Client객체와 함께 노티피케이션을 발송하였습니다.
• MainViewController 에서 Observer를 등록하여 노티피케이션이 오면 view가 업데이트 될 수 있도록 구현했습니다.

7. DispatchQueue에서의 스레드 생성 문제

• 10명을 추가하는 기능이 생기면서, 기존 로직의 경우 고객이 추가될 때 은행원 수 만큼 스레드를 만들다 보니 고객을 추가할 때 마다 계속해서 스레드를 3개씩 추가로 생성해내는 문제가 생겼습니다.
• 해당 문제를 해결하기 위해 DispatchQueue를 OperationQueue로 변경하고 OperationQueue를 업무타입의 수만큼 만들어 각각 maxConcurrentOperationCount 프로퍼티를 은행원 수에 맞게 설정하여 스레드를 전체 은행원 수 만큼만 유지할 수 있도록 해결하였습니다.

---

감사합니다!!

[Monsteel]

수고하셨습니다 파프리 마이노~!~!
조금 더 빠르게 리뷰를 남기려고 했지만, 생각보다 바쁜 금요일을 보내버려서.. 리뷰가 늦었습니다 🙏

더 뭔가 도움을 많이 드리고 싶었는데, 두분이 너무 잘해주셔서 그런지...ㅎ 별로 많은 도움을 드리지 못한것 같아서 조금 아쉽습니다 ㅜ
부족하지만 저의 리뷰가 여러분의 성장에 조금이라도 도움이 되셨으면 좋겠네요🙏

마지막도 코드에 대한 리뷰는 크게 없고 간단한 리뷰 몇가지만 남겨보았습니다. 확인 부탁드려요.

제 의견을 여쭈어보신 부분에 대해 의견을 남겨보자면!

1. Xcode 프로젝트 관리 구조
   기존에 구현한 ConsoleApp 프로젝트의 파일들이 UIApp을 구현하는데에도 필요했습니다.
   ConsoleApp에서 reference로 LinkedList.swift , Queue.swift 를 가져오고 나머지 파일은 구조의 변경이 있을 거라 예상되어 UIApp 프로젝트에서 직접 생성하였습니다.
   이 방법에 대해 토니는 어떻게 생각하시나요?

일단 이러한 경우, 일반적인 모듈화 방식(static library, static framework)으로 모듈을 분리해서 두 프로젝트에서 같은 모듈을 바라보는 방식으로 구현할수도 있습니다.

다만, Core구현부(LinkedList.swift , Queue.swift)를 그대로 가져오시고, UIApp 프로젝트에서 직접 생성하셔서 사용하신 그 의도를 높이 평가합니다. 좋은 고민을 하신것 같아요. 👏👏

2. Timer 실행 시 딜레이
   Timer의 timeInterval을 0.001초로 진행하는 경우 실제 시간과 거의 2 ~ 3초 차이가 나는 현상이 발생했습니다.
   우선 해당 값을 0.003초로 진행하여 현재는 0.2~3초 차이가 나도록 임시방편으로 두었습니다.
   해당 부분은 크게 고민하지 않고 넘어가도 되는 부분일까요? 아니면 저희가 놓친 부분이 있는지 궁금합니다!

음 이 부분은 저도 경험해보지 않은 포인트라 명확하게 어떻다라고 말씀을 드리기는 어려운 부분인데요,
별도의 환경에서 테스트해본 결과, 0.001초로 timeInterval을 실행했을 때 실제 시간과 거의 2 ~ 3초 차이나는 현상을 확인하지는 못했습니다. 이로 미루어 보아, 타이머만의 문제는 아닌것 같고, 뭔가 다른 요인에 의해발생되는 문제는 아닌가..! 🤔 하고 추측해 봅니다.🙌

혹시나 모르니, 파프리와 마이노도 별도의 환경에서 한번 타이머를 테스트해보시면 좋을것 같아요 🙌

파프리 마이노 모두 은행창구 메니저 프로젝트 진행하시느라 수고많으셨습니다~! ❤️

확인하시고 코멘트 주시면 approve 후 merge할 수 있도록 하겠습니다!

[Monsteel]

Base ~~ 패턴은 하위 뷰가 구현상속을 위해 사용하는 것인데,
클래스를 기반으로 구현상속을 하게 되면 리스코프 치환 법칙이 깨지는 경우도 많고, 객체 구조가 경직되는 경향이 있어서, 지양하는 패턴중 하나입니다.

클래스 상속 대신, protocol을 선언하고, 해당 protocol에 기본 구현을 extension으로 적어서, mixin 과 같이 사용하거나,
별도의 객체로 분리해서 composition(over inheritance)으로 중복을 제거하여 사용하는 일반적인 대안도 있습니다. 👍

[Mino777]

저희가 BaseUIView를 만들었던 이유로는 CustomView를 만들어줄 때 마다 이니셜라이저를 정의하는 반복적인 작업을 제거해주기 위함이었습니다.
그러려면 UIView로 부터 꼭 이니셜라이저가 오버라이딩되어야하는데 해당 부분을 protocol로 어떤식으로 진행해야되는지 감이 잘 오지 않는 것 같습니다😭
해당 부분 조금만 더 설명 부탁드려도 될까요?!
감사합니다!!

[Monsteel]

@Mino777 답변이 늦어 죄송합니다. 음.. Base라는 네이밍을 버리고, BandManagerUIApp.UIView 형태로 사용할 수 있도록 하는것도 하나의 방법일것 같아요. 구현방법보다는, 리스코프 치환 법칙이 깨지고, 객체 구조가 경직되는 현상이 발생한다는 것에 공감대가 생겼으면 하는 바램이고, 이 현상을 해결할 수 있는 방법들은 무수히 많아서, 어떻게 하면 해결할 수 있을지 고민해보면 좋을것 같아요! ✍🏻

수고하셨습니다 마이노, 파프리!

cc @papriOS

[Monsteel]

송신부와 수신부가 명확해 delegate를 통한 구현을 할수도 있었을것 같은데, NotificationCenter를 사용하신 이유가 있을까요? 🤔

[papriOS]

Notification Center를 실제로 경험해 본 적이 없고, 코드가 간결할 것 같아 사용해보았습니다!
현재는 프로젝트 규모가 작아 관리가 쉬웠지만 추후 notification을 post하는 곳이 많아지면 디버깅이 어려워지는 등 관리가 어려워진다는 점을 고려해 말씀해주신 것처럼 delegate를 통해 구현하는 것으로 수정했습니다!🙏

[Monsteel]

추가로 상속받지 않는 class에 대해 final을 적어주시는 부분 굉장히 좋습니다 🚀

클래스는 보통 dynamic dispatch를 채택하지만, 이렇게 final 키워드를 사용해서 서브클래싱을 막은 경우, 클래스여도 static dispatch(일반적으로 struct가 사용하는 method dispatch방법 중 하나입니다) 가능해서 성능적인 이점이 발생할 수 있습니다!