Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

은행 창구 관리 앱 [Step4] 댄, 네로 #95

Open
wants to merge 53 commits into
base: d_Nero
Choose a base branch
from

Conversation

eensungkim
Copy link

Step 4
멤버 : @eensungkim, @ODDNERO
리뷰어 : @yim2627

은행 창구 관리 앱 프로젝트의 마지막 스텝입니다!
정리하여 PR 드립니다. 잘 부탁드립니다! 🙇

PR 상세

  • 코드를 사용한 UI 구현
  • 비동기 데이터 UI 반영
  • 고객 추가 기능 구현
  • 초기화 기능 미구현 (:cry:)

이번 스텝을 통해서 배운 점

  • 코드를 사용한 UI 구현
  • 비동기 데이터를 활용한 UI 작업
  • 버튼 활용 시 addTarget 을 활용한 연결
  • semaphore 의 공유
  • 프로퍼티 감시자의 활용
  • Timer 클래스의 활용
  • 커스텀뷰의 활용
  • 스크롤뷰의 활용
  • completion Handler 의 학습과 활용

구성

Model

BankManager.swift

  • 기존 OperationQueue 에서 DispatchQueue 를 활용하는 것으로 변경
  • 고객 추가 기능 및 대기중/업무중 고객 리스트 추가

View

MainView.swift

  • 다양한 스택뷰를 조합하여 메인 뷰 구성
  • 대기중 스택뷰의 경우 스크롤뷰로 감싸 화면을 넘어가는 경우 스크롤되도록 구현

CustomerView.swift

  • 대기중/업무중에 들어가는 고객 커스텀뷰

Controller

ViewController.swift

  • 고객 추가 및 초기화 버튼 클릭 시 호출되는 메서드 관리
  • 타이머 기능 관리

질문

  1. 타이머를 센터로 맞춰놨더니 숫자의 너비에 따라 타이머가 깜박거리는 현상이 있었습니다.
    별도의 UIView 로 감싸주고 해당 뷰의 위치를 조정해주는 것으로 해결하려 했으나, 시간에 쫓긴 나머지 제대로 구현을 하지 못해 원상복구를 할 수 밖에 없었는데요.

    센터가 아닌 임의의 위치를 지정하게 되면 서로 다른 기기에서 사용하는 경우를 가정할 때, 반응성 있는 UI가 아니게 된다는 생각도 들었습니다.
    이 문제를 해결하기 위해서는 혹시 어떤 아이디어를 가지고 접근해야 할까요?

  2. 초기화를 구현하기 위해 고객이 추가되어 있는 커스텀큐에서 clear() 를 호출하여 커스텀큐를 빈 큐로 만들어 주었습니다.
    커스텀큐가 자체 프로퍼티로 갖고 있는 세마포어를 활용한다면, 문제가 없을 것으로 예상했었는데요.

    이렇게 초기화를 진행하게 되면 semaphore.wait()semaphore.signal() 이 짝지어지지 않은 상태에서 비동기 과정이 종료되면서 스레드가 semaphore.wait() 상태에서 멈춰 더 이상 실행이 되지 않는 문제가 있었습니다.
    semaphore 를 새롭게 지정해주었더니, 세마포어를 사용하는 도중에 deallocated 되어서 앱이 중단되는 문제가 있기도 했습니다.

    현재의 구성을 보면, 초기부터 예금고객큐와 대출고객큐를 별도로 관리하고 있습니다.
    그리고 while 문을 돌면서 각각의 큐에서 고객 노드를 뽑아 banker 에게 일을 맡기는 과정이 비동기 task(DispatchWorkItem) 으로 전달되고 있습니다.
    그리고 각각의 비동기 task 에서는 전달받은 고객 노드를 banker 에게 전달할 때, 이를 다시 DispatchWorkItem 으로 감싸 개별적인 커스텀 디스패치큐(depositQueue/loanQueue) 전달하고 있는데요.

    Banker 가 여전히 print 를 통해 메시지를 보여주기 때문에, 해당 로그를 통해 작업이 잘 마무리되었는지 추적이 가능했습니다.
    실제로도 "초기화" 버튼이 눌려 메서드가 실행되는 시점에, 이미 진행중이던 Banker 의 작업은 모두 잘 마무리가 되는 것을 확인할 수 있었습니다.
    해당 DispatchWorkItem 이 잘 동작했다면 signal() 또한 잘 동작했을 거라고 기대할 수 있다고 생각합니다.
    그러나 앞서 말한 것처럼 semaphore.wait() 이슈를 해결하지 못해 초기화를 결국 구현하지 못한 상황입니다.
    이 문제를 해결하기 위해 어떤 접근을 진행하면 좋을지 궁금합니다.

ODDNERO and others added 30 commits February 6, 2024 14:12
OperationQueue.isSuspended 가 예상대로 동작하지 않음.
DispatchSemaphore 로 입력과 출력을 동시 관리할 필요성
@yim2627 yim2627 self-requested a review February 9, 2024 09:26
Copy link

@yim2627 yim2627 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 댄, 네로

이번 스텝도 고생 많으셨습니다.

프로젝트 실행 결과 문제가 되는 부분이 조금 있어 이 부분도 확인 부탁드립니다.

화면 기록 2024-02-09 오후 7 00 42

한가지 말씀드리자면, Timer는 RunLoop에서 동작합니다. RunLoop는 사용자의 인터렉션을 관리하기 때문에, 인터렉션 발생시 타이머의 동기이벤트가 모두 씹혀버리고, 인터렉션이 끝났을 때 몰아서 처리됩니다. 이에 대한 해결 방법을 고민해보시고, RunLoop에 대해서 공부해보시는 것도 정말 좋을 것 같습니다.

image

타이머를 센터로 맞춰놨더니 숫자의 너비에 따라 타이머가 깜박거리는 현상이 있었습니다.
별도의 UIView 로 감싸주고 해당 뷰의 위치를 조정해주는 것으로 해결하려 했으나, 시간에 쫓긴 나머지 제대로 구현을 하지 못해 원상복구를 할 수 밖에 없었는데요.
센터가 아닌 임의의 위치를 지정하게 되면 서로 다른 기기에서 사용하는 경우를 가정할 때, 반응성 있는 UI가 아니게 된다는 생각도 들었습니다.
이 문제를 해결하기 위해서는 혹시 어떤 아이디어를 가지고 접근해야 할까요?

CenterX에 맞추든 양옆에 맞추든 해당 레이블의 크기는 숫자 크기에 의해 매번 달라질 것이고, 오토레이아웃은 달라지는 레이블의 크기에 맞게 조절하면서 레이블이 움직이는 현상이 발생하는 것이므로 이 부분은 디자인적으로 이슈가 있다고 생각이 듭니다.

만일 해결한다면, 고정된 리터럴값인 "업무 시간" 레이블과 시간이 나오는 레이블을 따로 두어 업무시간 레이블은 왼쪽에 고정시켜두되 타이머 레이블의 크기가 커질 수록 Constant를 줄여나가는 형식(lessThan)이나 타이머 레이블의 사이즈를 고정시켜 Text의 크기가 달라지도록 하는 방법이 있을 것 같네요.

초기화를 구현하기 위해 고객이 추가되어 있는 커스텀큐에서 clear() 를 호출하여 커스텀큐를 빈 큐로 만들어 주었습니다.
커스텀큐가 자체 프로퍼티로 갖고 있는 세마포어를 활용한다면, 문제가 없을 것으로 예상했었는데요.
이렇게 초기화를 진행하게 되면 semaphore.wait() 과 semaphore.signal() 이 짝지어지지 않은 상태에서 비동기 과정이 종료되면서 스레드가 semaphore.wait() 상태에서 멈춰 더 이상 실행이 되지 않는 문제가 있었습니다.
semaphore 를 새롭게 지정해주었더니, 세마포어를 사용하는 도중에 deallocated 되어서 앱이 중단되는 문제가 있기도 했습니다.
현재의 구성을 보면, 초기부터 예금고객큐와 대출고객큐를 별도로 관리하고 있습니다.
그리고 while 문을 돌면서 각각의 큐에서 고객 노드를 뽑아 banker 에게 일을 맡기는 과정이 비동기 task(DispatchWorkItem) 으로 전달되고 있습니다.
그리고 각각의 비동기 task 에서는 전달받은 고객 노드를 banker 에게 전달할 때, 이를 다시 DispatchWorkItem 으로 감싸 개별적인 커스텀 디스패치큐(depositQueue/loanQueue) 전달하고 있는데요.
Banker 가 여전히 print 를 통해 메시지를 보여주기 때문에, 해당 로그를 통해 작업이 잘 마무리되었는지 추적이 가능했습니다.
실제로도 "초기화" 버튼이 눌려 메서드가 실행되는 시점에, 이미 진행중이던 Banker 의 작업은 모두 잘 마무리가 되는 것을 확인할 수 있었습니다.
해당 DispatchWorkItem 이 잘 동작했다면 signal() 또한 잘 동작했을 거라고 기대할 수 있다고 생각합니다.
그러나 앞서 말한 것처럼 semaphore.wait() 이슈를 해결하지 못해 초기화를 결국 구현하지 못한 상황입니다.
이 문제를 해결하기 위해 어떤 접근을 진행하면 좋을지 궁금합니다.

우선 Controller나 Manager 등의 객체에서 생성, 관리할 객체에서 Semaphore를 관리하는 것이 아닌 Queue 객체에서 관리하는 것은 Queue의 역할에 어울리지 않는다는 생각이 듭니다. Semaphore를 Queue 내부에서 갖고 있게되면, DispatchGroup 등을 통한 동기화도 골치가 아파지기 떄문에 현재 하신 방식이 맞는 것 같습니다.

다만 Clear가 동작하지 않는 것은, 말씀해주신대로 카운트 관리가 잘못되어 DeadLock에 놓여지는 것으로 파악됩니다. 이 문제는 작업 취소 및 초기화시 내렸던 카운트를 다시 올려주는 것을 보장하도록 만들어준다면 정상 동작할 것으로 보입니다.

isa = PBXGroup;
children = (
C7694E3A259C3E9F0053667F /* AppDelegate.swift */,
C7694E3E259C3E9F0053667F /* ViewController.swift */,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

목적에 맞는 네이밍으로 수정부탁드립니다

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MainViewController 로 수정했습니다.


import UIKit

class ViewController: UIViewController {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

상속하지 않으니 final을 붙여도 되겠네요

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수정했습니다!


class ViewController: UIViewController {
private let mainView: MainView
private let dataSource: BankManager
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dataSource라는 네이밍은 어색한 것 같습니다. 차라리 manager가 나을 것 같아요.

Copy link
Author

@eensungkim eensungkim Feb 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

뷰에서 보여줘야 하는 데이터를 관장하고 있으니 dataSource 가 더 어울린다고 생각했었는데요.
말씀해주신 내용을 듣고 고민해보니 데이터만 다루는 것이 아니라, 실제로 내부적인 로직이 돌아가고 있고 여기에 따른 결과가 반영이 되고 있다는 점을 감안하면 manager 가 더 맞을 것 같습니다.
반영하여 수정했습니다!

self.mainView = MainView()
self.dataSource = dataSource
super.init(nibName: nil, bundle: nil)
dataSource.delegate = self.mainView
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

manager의 delegate 객체가 View가 되는 것은 View의 책임에 어울리지 않는 것 같은데, 어떻게 생각하시나요?
delegate로 처리하지 않고, ViewController가 mainView를 Configure 해주는 것이 각자 책임에 어울릴 것 같습니다.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

View 의 책임에 어울리지 않는다는 점에 동의합니다.
일단 동작하게 만들고 나서 리팩토링하자는 마음이었는데 시간이 모자라면서 챙기지 못했습니다. 😢

Comment on lines 32 to 58
@objc func addCustomerButtonTapped() {
countElapsedTime()
dataSource.addCustomer()
dataSource.startBankingProcess()
}

@objc func resetService() {
dataSource.reset()
if let timer = stopwatch {
timer.invalidate()
stopwatch = nil
elapsedTime = 0
mainView.setTimer("업무시간 - 00:00:000")
}
}

func countElapsedTime() {
stopwatch = Timer.scheduledTimer(withTimeInterval: 0.005, repeats: true) { timer in
self.elapsedTime += timer.timeInterval
let minutes = Int(self.elapsedTime) / 60
let seconds = Int(self.elapsedTime) % 60
let milliseconds = Int(self.elapsedTime * 1000) % 1000

let timeString = String(format: "%02d:%02d:%03d", minutes, seconds, milliseconds)
self.mainView.setTimer("업무시간 - \(timeString)")
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

현재 구조가 CustomView가 ViewController에 직접 접근하여 버튼 액션에 대한 기능을 사용하고 있는데, ViewController의 메서드들을 접근제한자 없이 공개적으로 사용하는 것 보단, View의 컴포넌트(버튼 등)을 오픈해놓은 것이 더 나을 것 같은데, 어떻게 생각하시나요?

Copy link
Author

@eensungkim eensungkim Feb 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드를 수정하면서 질문 주실때와 상황이 달라진건지 제가 조금 헷갈립니다만,
현재는 접근제한자를 붙여 MainViewController 안에서만 사용하도록 변경해두었습니다! 🙏


extension BankManager {
func startBankingProcess() {
let depositTask = DispatchWorkItem { [self] in
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[self]는 무엇인지 설명부탁드립니다.
그리고 weak가 아닌 strong으로 캡쳐하셨는지 이유가 궁금합니다.

Copy link
Author

@eensungkim eensungkim Feb 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[self] 는 캡처리스트입니다. 해당 클로저가 실행되는 시점에 자신이 속한 인스턴스를 인식하기 위해 self 캡처를 진행합니다.
일반적으로 클래스 인스턴스의 경우 reference counting 을 줄이기 위해 weak 또는 unowned 를 붙여 참조가 늘어나지 않도록 해주는 것으로 알고 있습니다.

strong 으로 캡쳐한 특별한 이유가 있지는 않았습니다.
비동기로 동작하게 될 depositTask 는 반드시 뱅크매니저 이후에 사라지게 된다는 점 때문에 강한 참조를 유지하더라도 문제가 없을 것으로 예상해서 strong 캡처를 진행했습니다.

weak 로 캡쳐하더라도 동작에 문제가 없고 메모리 점유 이슈가 사라지는 만큼 weak 로 수정했습니다!

}

if !isDepositQueueRunning {
DispatchQueue.global().async(group: depositGroup, execute: depositTask)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

메인스레드와 글로벌 스레드의 차이는 무엇일까요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

메인 스레드는 앱이 실행될 때 가지게 되는 기본적인 실행 흐름이며 serial 하게 동작합니다. 화면을 그리는 모든 작업들은 메인 스레드에서 실행되어야 합니다.
글로벌스레드는 concurrent 한 스레드이며, 백그라운드 스레드입니다.

Comment on lines 136 to 137
// depositQueue.cancelAllOperations()
// loanQueue.cancelAllOperations()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

사용하지 않는 주석은 제거해주세요!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수정했습니다!

Comment on lines 49 to 57
stopwatch = Timer.scheduledTimer(withTimeInterval: 0.005, repeats: true) { timer in
self.elapsedTime += timer.timeInterval
let minutes = Int(self.elapsedTime) / 60
let seconds = Int(self.elapsedTime) % 60
let milliseconds = Int(self.elapsedTime * 1000) % 1000

let timeString = String(format: "%02d:%02d:%03d", minutes, seconds, milliseconds)
self.mainView.setTimer("업무시간 - \(timeString)")
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

타이머는 어떻게 동작할까요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Timer 클래스는 일정한 시간이 지날 때마다 타겟이 되는 객체에 메시지를 전송합니다.
위 코드에서는 0.005초가 지날 때마다 기준이 되는 elapsedTime 에 지나간 시간을 초 기준으로 누적한 뒤, 이를 환산하여 뷰에 반영하고 있습니다.

말씀해주신 RunLoop 는 추가 공부가 필요한 것 같아 문서를 읽는 중입니다.
말씀해주신대로 인터랙션 발생시 의도한 대로 동작하지 않고 타이머 이벤트가 씹혀버리는 문제가 핵심이라고 판단하여,
타이머와 유사하게 동작하는 흐름을 별도의 비동기 스레드로 던져서 처리하는 방향으로 수정해 보았습니다.

Comment on lines 33 to 35
countElapsedTime()
dataSource.addCustomer()
dataSource.startBankingProcess()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이렇게 되면 초기화 없이 고객을 추가할 경우 stopwatch에 타이머가 계속 할당되지 않나요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

타이머를 아예 새롭게 수정하여 문제를 해결했습니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants