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

Step3 : 업무 중 대기 (리나) #19

Merged
merged 21 commits into from
Jan 14, 2021

Conversation

lina0322
Copy link

안녕하세요~🙇🏻‍♀️

[Step3]

  1. HeadOffice를 싱글턴으로 생성

    • 본사는 1개라고 생각했습니다!
  2. Teller는 Client의 BusinessType에 따라 handleLoan 또는 handDeposit 호출

    • handLoan에서 HeadOffice의 queue에 접근
    • Teller는 HeadOffice가 대출 심사를 끝낼 때까지 semaphore을 이용하여 대기
  3. step3에 필요한 문구가 추가되었고, 다시보니 고치면 좋을 것 같은 것도 수정해보았습니다!

[고민]

기존에 만들어두었던 enum 중 BusinessType은
고객이 어떤 업무를 보는지 가지고 있는 형태로, 은행원이 확인하고 이에 따른 행동을 하기 위해 만든 enum이었습니다.
여기에 시간 정보도 담고 있었는데요.

기존에 있던 대출 1.1초가 0.3초, 0.3초, 0.5초로 나뉘어져 동작을 하는데
0.3초 두번은 Teller가 업무를 보는 것이라 동일하게 생각해 2번 동작하고,
0.5초는 본사가 진행하는 것이라, HeadOffice가 가지고 있으면서 사용합니다.

그런데 이 0.3초 두번과 0.5초 한번을 따로 관리해야할지...
0.3초 두번이 은행원이 시간은 똑같이 쓰지만 다른 행동으로 봐야할지;;
지금과 같은 방법으로 진행해도 괜찮을지 고민입니다🤦🏻‍♀️

리뷰부탁드립니다!
감사합니다😄

@AppleCEO AppleCEO self-requested a review January 12, 2021 13:38
Copy link
Member

@AppleCEO AppleCEO left a comment

Choose a reason for hiding this comment

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

고생하셨습니다.
몇가지 코멘트 남겨놨습니다.
추가로 두가지 질문 더 드릴께요.

  1. client 는 구조체인데 init 함수를 따로 만들어준 이유가 있나요?
  2. initTellers 는 Clients 에서 client 들을 init 하는 것처럼 Tellers 객체를 만들어서 선언하지 않은 이유가 있나요?

Comment on lines -37 to +38
for teller in self.tellers {
if self.clients.count == 0 {
for teller in tellers {
if clients.count == 0 {
Copy link
Member

Choose a reason for hiding this comment

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

self. 를 지운 이유는 무엇인가요?

Comment on lines 15 to 22
guard let businessType = BusinessType.allCases.randomElement(), let priority = Client.Priority.allCases.randomElement() else {
return
}
list.append(Client(waitingNumber: waitingNumber, businessType: businessType, priority: priority))
let clinet = Client(waitingNumber: waitingNumber, businessType: businessType, priority: priority)
list.append(clinet)
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

보통 하나의 파일에 하나의 타입만을 저장합니다.
Clients 를 따로 파일로 빼내는 것은 어떨까요?

@@ -22,6 +22,8 @@ enum Message {
static let close = "업무가 마감되었습니다. 오늘 업무를 처리한 고객은 총 %d명이며, 총 업무시간은 %.2f초입니다."
static let tellerStart = "%d번 %@고객 %@업무 시작"
static let tellerFinish = "%d번 %@고객 %@업무 완료"
static let loanStart = "%d번 %@고객 %@심사 시작"
static let loanFinish = "%d번 %@고객 %@심사 완료"
}

enum BusinessType: CaseIterable, CustomStringConvertible {
Copy link
Member

Choose a reason for hiding this comment

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

보통 하나의 타입을 하나의 파일에 저장합니다. enum 들도 따로 파일로 저장하는 건 어떨까요?

continue
}

switch command {
case .start:
let randomNumber = Int.random(in: minClientCount...maxClientCount)
let clients = Clients.init(count: randomNumber)
let clients = Clients(count: randomNumber)
bank.operateBank(teller: tellerCount, client: clients.list)
Copy link
Member

Choose a reason for hiding this comment

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

bank.operateBank 라는 부분은 조금 어색한 것 같네요.
은행이 은행을 운용한다는 뜻으로 보이는데 조금 개선해볼 수 있을까요?

@lina0322
Copy link
Author

lina0322 commented Jan 12, 2021

@AppleCEO

안녕하세요 도미닉! 매번 빠른 피드백 감사합니다.🙇🏻‍♀️

[1번]

client 는 구조체인데 init 함수를 따로 만들어준 이유가 있나요?

step1에서는 고객의 등급은 일반 (이것은 저의 추측..! 어쨌든 등급이 없었습니다) , 업무는 예금이었습니다.
이 형태가 기본이라고 생각하고, 혹시 모를 상황을 대비하여(?)
따로 등급이나 업무를 지정해주지 않아도 고객을 생성할 수 있도록 만들어두었습니다.

[2번]

initTellers 는 Clients 에서 client 들을 init 하는 것처럼 Tellers 객체를 만들어서 선언하지 않은 이유가 있나요?

제 머릿속 로직상에는 teller는 bank에 의해서 고용되는 것이고, clients는 외부에서 들어오는 것이라는 생각이 들었습니다.
그래서 teller는 bank자체에서 생성하고, clients는 만들어진 것이 들어오는걸로 작성하였는데

매 실행마다 teller가 새로 init되는 것이 이상하다는 생각이 들어서, 커멘트 단 후에 추가로 다시 수정하였습니다!

[3번]

self. 를 지운 이유는 무엇인가요?

그 곳에서 dispatchQueue의 사용을 시도할때 입력했다가 지우지 않았었는데,
굳이 명시할 필요가 없다고 판단하여 이번에 삭제했습니다!

[4번]

bank.operateBank 라는 부분은 조금 어색한 것 같네요.
은행이 은행을 운용한다는 뜻으로 보이는데 조금 개선해볼 수 있을까요?

네이밍은 항상 어렵네요💦
일단 open으로 변경해보았는데, 혹시 이것도 좀 어색할까요..?

추가 수정

  • 댓글 작성 후에 추가로 수정을 조금 더 했습니다.
  1. Tellers 객체 생성하여, 거기서 teller 생성
  2. closeBank의 기능과 이름을 변경했습니다.
    • 추가고민: 변경된 resetFinishedClientCount에는 finishedClientCount = 0 기능만 있는데, 앞으로 bank를 닫을때 다른 기능이 생길수도 있다고 생각해 1개의 기능이지만 함수로 남겨두었습니다. 어떻게 생각하시는지 여쭤보고싶습니다!!
  3. handleLoan이 복잡하다고 생각되어서, 서류 확인 - 본사에 서류 전달 - 대출 마무리의 3개 함수로 변경했습니다.

하단의 추가 커밋도 확인 부탁드리겠습니다!!
감사합니다🙏

@AppleCEO AppleCEO merged commit 00b868e into yagom-academy:1-lina Jan 14, 2021
@AppleCEO
Copy link
Member

놓쳤었네요. 고생하셨어요~

@AppleCEO
Copy link
Member

  1. 기본값을 넣는 다른 방법은 뭐가 있을까요?
  2. open 이 더 나을 것 같네요 ㅎㅎ

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.

None yet

2 participants