은행창구 관리 앱 [Step3] Harry #101
Conversation
There was a problem hiding this comment.
안녕하세요 @hemil0102 ~! 제가 리뷰가 늦었네요 ㅎㅎ.
변경내역만 보기 위해 https://github.com/tasty-code/ios-bank-manager/pull/90/files 를 참고해서 바뀐 부분들에만 코멘트를 간단하게 남겨보았어요!
확인해보시고 궁금한게 있으시다면 DM 주세요! 😄
고생하셨고 기다려주셔서 감사합니다 🎉
|
|
||
| while isOpen { | ||
| executeBankingOperation() | ||
| operate() |
There was a problem hiding this comment.
재귀호출을 쓰셨던 이유에 대해 답변주신 내용 읽어보았습니다~!
이 함수를 보면 재귀 호출이 여러곳에서 일어나고 있는데요!
함수호출의 흐름을 잘 따라가고 제어할 수 있을지 고민해주시면 좋을 것 같습니다.
재귀를 쓸 때 가장 중요한건 base case, recursive case -> 즉 재귀가 끝나는 지점과 반복되는 지점이 명확히 구분되어야한다고 생각합니다. 그래야 메모리 문제 없이 잘 쓸 수 있으니까요!
재귀가 끝나는 지점을 앞쪽에 두고 / 반복되는 조건을 체크한 뒤 뒤에서 재귀하는 형태로 만들어보시는걸 추천드릴게요!
| customerNumber.deposit = depositTicketMachine.totalLength() | ||
| } | ||
|
|
||
| func createCustomers() { |
There was a problem hiding this comment.
이렇게 내부에서만 호출되는 함수들에는 접근 제어자 붙여주시면 좋을 것 같아요!
| @@ -0,0 +1,5 @@ | |||
|
|
|||
| struct CustomerNumber { | |||
There was a problem hiding this comment.
CustomerNumber라는 타입을 새로 만드신 이유가 있을까요~?
제가 봤을 때에는 2개의 프로퍼티로 따로 두어도 될 것 같아서요!
|
|
||
| let bankingServiceStart = Date.timeIntervalSinceReferenceDate | ||
| handleCustomersTasks() | ||
| let bankingServiceEnd = Date.timeIntervalSinceReferenceDate |
There was a problem hiding this comment.
이 부분에 대해 제가 말씀드리고자 했던 것은 아래와 같습니다.
Date()가 성능상 이점이 있다거나 하는 것은 아닙니다. 다만 handleCustomersTasks() 호출 전후로 아래와 같이 로직을 설계해도 결과는 동일하기 때문에 말씀드린것이었습니다.
let bankingServiceStart = Date()
handleCustomersTasks()
let bankingServiceEnd = Date()2001.1.1.00:00:00 시간과의 차이값을 두번 가져오는 것이 읽는 입장에서는 특정 의도가 있는 것인가 하는 생각이 들 수 있을 것 같아요!
|
|
||
| struct Employee { | ||
| var loanTask: DispatchQueue? | ||
| var depositTask: DispatchQueue? |
There was a problem hiding this comment.
한명의 종사자는 하나의 Task만 처리할 수 있으므로 두 DispatchQueue를 가지는건 조금 어색한 것 같아요.
Task는 하나만 두고 해당 종사자가 어떤일을 처리할 수 있는지는 BankingService로 구분하는게 어떨까요?
| bankManager.employees[index].depositTask?.async(group: group) { | ||
| semaphore.wait() | ||
| while let customer = customerDepositQueue.dequeue() { | ||
|
|
| func handleLoanTasks(atTable index: Int, with customerLoanQueue: Queue<Customer>, bankManager: BankManager, group: DispatchGroup) { | ||
| bankManager.employees[index].loanTask?.async(group: group) { | ||
| while let customer = customerLoanQueue.dequeue() { | ||
| guard let (customerTicketNumber, customerBankingService) = customer.askEmployeeHandleTasks(), let definedCustomerTicketNumber = customerTicketNumber, let definedCustomerBankingService = customerBankingService else { return } | ||
| print("🌝 \(definedCustomerTicketNumber)번 고객 \(definedCustomerBankingService.name)업무 시작") | ||
| Thread.sleep(forTimeInterval: 1.1) | ||
| print("🌝 \(definedCustomerTicketNumber)번 고객 \(definedCustomerBankingService.name)업무 종료") | ||
| } | ||
| } | ||
| } | ||
|
|
||
| func handleDepositTasks(atTable index: Int, with customerDepositQueue: Queue<Customer>, bankManager: BankManager, group: DispatchGroup, semaphore: DispatchSemaphore) { | ||
|
|
||
| if index == 1 { | ||
| bankManager.employees[index].depositTask?.async(group: group) { | ||
| semaphore.wait() | ||
| while let customer = customerDepositQueue.dequeue() { | ||
|
|
||
| guard let (customerTicketNumber, customerBankingService) = customer.askEmployeeHandleTasks(), let definedCustomerTicketNumber = customerTicketNumber, let definedCustomerBankingService = customerBankingService else { return } | ||
| semaphore.signal() | ||
|
|
||
| print("🥵 \(definedCustomerTicketNumber)번 고객 \(definedCustomerBankingService.name)업무 시작") | ||
| Thread.sleep(forTimeInterval: 0.7) | ||
| print("🥵 \(definedCustomerTicketNumber)번 고객 \(definedCustomerBankingService.name)업무 종료") | ||
| } | ||
| } | ||
| } else if index == 2 { | ||
| bankManager.employees[index].depositTask?.async(group: group) { | ||
| semaphore.wait() | ||
| while let customer = customerDepositQueue.dequeue() { | ||
| guard let (customerTicketNumber, customerBankingService) = customer.askEmployeeHandleTasks(), let definedCustomerTicketNumber = customerTicketNumber, let definedCustomerBankingService = customerBankingService else { return } | ||
| semaphore.signal() | ||
|
|
||
| print("🥶 \(definedCustomerTicketNumber)번 고객 \(definedCustomerBankingService.name)업무 시작") | ||
| Thread.sleep(forTimeInterval: 0.7) | ||
| print("🥶 \(definedCustomerTicketNumber)번 고객 \(definedCustomerBankingService.name)업무 종료") | ||
| } |
There was a problem hiding this comment.
잘 만들어주셨는데요!
Employee라는 타입 안에는 단 하나의 DispatchQueue와 하나의 handleTask만 있는 형태로 추상화를 하는게 좋지 않을까요??
현재 형태는 Employee타입이 너무 많은 정보를 알고 있는것 같습니다.
| @@ -0,0 +1,25 @@ | |||
|
|
|||
| class Queue<T: Equatable> { | |||
| Employee(loanTask: DispatchQueue(label: "대출1", attributes: .concurrent)), | ||
| Employee(depositTask: DispatchQueue(label: "예금1", attributes: .concurrent)), | ||
| Employee(depositTask: DispatchQueue(label: "예금2", attributes: .concurrent)) |
There was a problem hiding this comment.
concurrent로 만들어주셨지만 사실상 async안에 들어가는 작업은 하나씩밖에 없어서 .serial로 만든 것이랑 동일하게 동작할 것 같은데, 한번 확인해보시면 좋을 것 같네요!
| self.employees = employees | ||
| } | ||
|
|
||
| func reportDeadlineSummary(with customerNumber: Int, startTime bankingServiceStart: TimeInterval, endTime bankingServiceEnd: TimeInterval) { |
There was a problem hiding this comment.
#90 (comment)
필요한 값이 아닌 인스턴스를 넘기게 되면 함수 내부에서는 인스턴스의 불필요한 정보까지 접근 가능하게 되는 측면이 있습니다.
함수의 형태를 좀 더 깔끔하게 유지시켜주기 위해 저는 필요한 값들만 따로 넘겨주도록 작성합니다 😄
(특정 인스턴스의 여러 값들에 접근해야 하는 경우는 또 생각해봐야겠죠??)
안녕하세요, 코든 @ictechgy
리뷰 진행 중이던 STEP3 PR 레포지토리가 삭제되어서 다시 남겨드립니다.
지난 상세 내역은 아래 링크 확인 부탁드립니다.
#90
감사합니다.