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

Step 2 : 우선순위와 다중처리 (리나) #15

Merged
merged 19 commits into from
Jan 12, 2021

Conversation

lina0322
Copy link

@lina0322 lina0322 commented Jan 10, 2021

[step2 구현]

  1. Client 정렬을 위해 Priority 생성 후 priority를 기준으로 Comparable extension하여 sorted 할 수 있도록 함
  2. BusinessType에 loan 추가
  3. Client를 생성할때 allCases.randomElement()를 사용하여 priority와 businessType을 랜덤하게 생성
  4. 은행원은 3명으로 증원

리뷰 부탁드립니다🙇🏻‍♀️

@AppleCEO AppleCEO self-requested a review January 11, 2021 12:51
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.

고생하셨습니다.
코드가 명료하네요.
몇가지 생각할 점들 남겨놨습니다.
화이팅입니다~

enum Priority: Comparable, CaseIterable {
case VVIP
case VIP
case normal
Copy link
Member

Choose a reason for hiding this comment

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

case 이름을 일반으로 하지 않은 이유가 있나요?

class Dashboard {
struct Dashboard {
Copy link
Member

Choose a reason for hiding this comment

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

구조체로 바꾼 이유는 무엇인가요?

Comment on lines -21 to 25
enum Message: String {
case close = "업무가 마감되었습니다. 오늘 업무를 처리한 고객은 총 %d명이며, 총 업무시간은 %.2f초입니다."
case tellerStart = "%d번 고객 업무 시작"
case tellerFinish = "%d번 고객 업무 완료"
enum Message {
static let close = "업무가 마감되었습니다. 오늘 업무를 처리한 고객은 총 %d명이며, 총 업무시간은 %.2f초입니다."
static let tellerStart = "%d번 %@고객 %@업무 시작"
static let tellerFinish = "%d번 %@고객 %@업무 완료"
}
Copy link
Member

Choose a reason for hiding this comment

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

case 을 static let 으로 바꾼 이유는 무엇인가요?

BankManagerConsoleApp/BankManagerConsoleApp/main.swift Outdated Show resolved Hide resolved
BankManagerConsoleApp/BankManagerConsoleApp/main.swift Outdated Show resolved Hide resolved
BankManagerConsoleApp/BankManagerConsoleApp/main.swift Outdated Show resolved Hide resolved
BankManagerConsoleApp/BankManagerConsoleApp/main.swift Outdated Show resolved Hide resolved
Comment on lines 40 to 51
private func initClients(_ number: Int) -> [Client]? {
var clients: [Client] = []

for waitingNumber in 1...number {
guard let businessType = BusinessType.allCases.randomElement(), let priority = Client.Priority.allCases.randomElement() else {
return nil
}

clients.append(Client(waitingNumber: waitingNumber, businessType: businessType, priority: priority))
}
return clients
}
Copy link
Member

Choose a reason for hiding this comment

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

메인에서 메소드를 저장하는 것보다 좋은 방법이 있을 것 같아요.
혹시 [Client]를 저장하는 Clients라는 객체를 만들어볼 수 있을까요?

@lina0322
Copy link
Author

@AppleCEO

안녕하세요 도미닉!🙇🏻‍♀️

[1번]

case 이름을 일반으로 하지 않은 이유가 있나요?

case 이름은.. 저에게 프로퍼티명이라고 생각되어서요!
이러한 부분에서, 한글을 사용하는게 어색하다고 느껴졌습니다.

[2번]

구조체로 바꾼 이유는 무엇인가요?

처음에는 싱글톤을 만들려고 했다가,
굳이 싱글톤이 아니라 타입 메소드로도 가능할 것 같아서 수정하는 과정에서 class를 struct로 변경하지 않았던 것 같습니다.
(상속이라던지 참조타입으로 굳이 만들 필요가 없으면 struct로 하는게 좋다고 들어서 발견 후 수정했습니다!)

++ 그런데 혹시 이 부분을 enum으로해서 fake name space(아래 3번에서 사용한 enum처럼?!)처럼 사용하는게 더 나을까요?
따로 인스턴스를 생성할 일이 없을 것 같아서요🧐

[3번]

case 을 static let 으로 바꾼 이유는 무엇인가요?

enum에서 case들을 사용할때 rawValue로 접근하거나, var description 등을 만들어서 사용하는 것보다
static let이 조금 더 사용하기 편해서 변경하였습니다!
또한 추가적으로 다음 step에서 해당 enum에 문구가 조금 더 추가될 예정인데,
case들이 유기적으로 연결되어있는 느낌은 아니여서 변경하게 되었습니다!

[4번]

메인에서 메소드를 저장하는 것보다 좋은 방법이 있을 것 같아요.
혹시 [Client]를 저장하는 Clients라는 객체를 만들어볼 수 있을까요?

저도 어색하다고 생각했던 부분인데, 이런 방법이 있었군요!
한번 만들어서 시도해봤는데, 도미닉이 저에게 준 힌트가 이 방향이 맞을까요..?!

리뷰해주셔서 감사합니다🙇🏻‍♀️

@AppleCEO
Copy link
Member

1번 - description을 프로퍼티명으로 많이 사용하셨던데 CustomStringConvertible 프로토콜을 활용해보는 건 어떨까요?
2번 - enum은 보통 case를 정의하므로 스타일에 따라 다르겠지만 저는 struct를 사용하는게 더 나은 것 같아요.
4번 - 네 제가 의도한게 맞네요!

DispatchQueue 와 한가지 더 기본적으로 제공되는 큐가 있는데 DispatchQueue 를 선택한 이유가 있을까요?

@lina0322
Copy link
Author

@AppleCEO

빠른 답변 감사합니다!!👍

[1번]

description을 프로퍼티명으로 많이 사용하셨던데 CustomStringConvertible 프로토콜을 활용해보는 건 어떨까요?

아, 제가 사용하고싶었던 description이 CustomStringConvertible을 채택해야 활용되는거였어요!!
말씀해주셔서 감사합니다!🙇🏻‍♀️
적용하는 김에 기억하기 위해 가능한 부분은 모두 사용해보았어요!

[추가]

DispatchQueue 와 한가지 더 기본적으로 제공되는 큐가 있는데 DispatchQueue 를 선택한 이유가 있을까요?

한 가지 더 기본적으로 제공되는 큐가 Operation Queue를 말씀하시는거겠죠?!

Operation Queue도 Dispatch Queue와 비슷한 기능을 하고, 내부적으론 Dispatch Queue 위에서 동작한다고 알고 있습니다.
다만 Operation Queue는 동시에 실행할 수 있는 동작의 최대 수 지정, 동작 일시 중지 및 취소의 기능을 더 가지고있고,
Dispatch Queue는 앞선 기능을 하지 못합니다.
대신 Operation Queue가 구현하기가 조금 더 복잡하고,
현재 스펙에서 Dispatch Queue로도 구현이 가능하다고 생각해서 Dispatch Queue를 사용했습니다!

@AppleCEO AppleCEO merged commit 76df0c8 into yagom-academy:1-lina Jan 12, 2021
@AppleCEO
Copy link
Member

고생하셨습니다~

TTOzzi pushed a commit that referenced this pull request May 8, 2022
* feat: 고객10명추가 버튼, 초기화 버튼을 view에 추가

* feat: businessHoursLabel를 view에 추가

* feat: waitLabel, workLabel을 BankView에 추가

* feat: CustomView 구현

* feat: customerStackView, waitStackView, workStackView view에 추가

* feat: baseStackView 구현 및 레이아웃 설정

* feat: CustomerView layout 설정

* refactor: baseStackView layout 조정, businessHoursLabel 컬러 변경

Co-authored-by: dudu <whqtkf12@naver.com>
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