-
Notifications
You must be signed in to change notification settings - Fork 37
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] 제인, 세제 #89
base: d_Sajae
Are you sure you want to change the base?
Conversation
작업당 시간 합산이 아닌 전체 작업시간 측정으로 변경
} | ||
} | ||
|
||
private func validateUserInput(with userInput: String?) throws -> Int? { | ||
private func validateUserInput(with userInput: String?) throws -> [Customer]? { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
validateUserInput 의 리턴값이 예측되지 않는거같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
validateUserInput()
에서 리턴값을 제거하여 리팩토링 진행했습니다!
|
||
static func makeCustomers(count: Int) -> [Customer] { | ||
var customers = [Customer]() | ||
for number in 1...count { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enumerate 와 forEach로 바꾼다면 어떻게 바꾸실건가요 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
조언 참고해서 forEach
와 비슷하지만 새로운 시퀀스를 만들어 반환하는 map
과 enumerated
를 함께 사용해서 바꿔보았는데
요 방법은 어떨까요??
static func makeCustomers(count: Int) -> [Customer] {
let services = (1...count).map { _ in BankService.allCases.randomElement() ?? .deposit }
let customers = services.enumerated().map { Customer(number: $0.offset + 1, service: $0.element) }
return customers
}
|
||
import Foundation | ||
|
||
struct MemberFactory { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
꼭 필요한가요 ? 조금 동떨어진 느낌이 드네요.
만드신 이유와 필요한 이유를 각각 말씀해주세요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재 Bank 객체에서 손님을 받고 있습니다.
이 때 Bank의 역할은 손님을 받을 뿐이지 손님을 생성하는 역할까지 맡기기에는 부적절한 것 같아
팩토리 패턴을 사용해 손님을 생성하는 객체를 따로 만들었습니다.
|
||
struct MemberFactory { | ||
|
||
static func makeCustomers(count: Int) -> [Customer] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
static으로 만드셨네요. 간편한 사용을 위한거라면 enum + static이 인스턴스 생성도 안할수 있는 선택지가 될거같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enum Static이 인스턴스 생성을 안한다는 부분에서 이점이 있군요!
수정해보고 적용시켜보겠습니다!
@@ -11,8 +11,8 @@ enum Message { | |||
case `default` | |||
case userInput | |||
case inputError | |||
case startTask(number: Int) | |||
case finishTask(number: Int) | |||
case startTask(Int, String) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
case startTask(Int, String)
너무 모호합니다. 무엇을 의미하는지 유추할수 있게 수정해주세요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
반영커밋: 298b933
await depositTasks | ||
await depositTasks2 | ||
let endTime = Date() | ||
return endTime.timeIntervalSince(start) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
performTotalTask 는 처리를 하는것과 -> Double 간의 상관관계가 나타나지 않은 이름인 것 같습니다.
예측 가능한 구조로 바꿔주세요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
performTotalTask
의 리턴값을 없애주어 함수명처럼 Task를 수행하기만 하고 전체작업시간은 변수를 따로 만들어주어 전달했습니다!
@@ -10,7 +10,7 @@ import Foundation | |||
final class Bank { | |||
|
|||
private let bankManager: BankManager | |||
private var customerCount: Int? | |||
private var customers: [Customer]? = nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
배열이라면 nil보다는 비어있다는게 조금 더 타당한 것 같은데요.
언래핑의 수고스러움도 덜 수 있구요. 혹시 옵셔널 + 기본값 nil을 사용하신 특별한 이유가 있을까요 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
customer 타입을 변환하는 과정에서 고민을 적게하고 똑같이 옵셔널로만 바꾸어주었습니다.
말씀하신 것처럼 nil보다는 비어있다는게 더 자연스러워
private var cutomers = [Customer]()
로 변경했습니다!
guard let customers = customers else { return } | ||
await alignCustomer(with: customers) | ||
await handleTask() | ||
await open() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
확실히 가독성이 좋아진 것 같네요 ~!
|
||
private var customerQueue = Queue<Int>() | ||
private let duration = 0.7 | ||
private var loanQueue = Queue<Customer>() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
struct 에서 actor로 바꾼후에도 var여야 하나요 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actor는 참조타입이기 때문에 let으로 선언했습니다!
} | ||
} | ||
|
||
struct Customer { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
꼭 필요한건 아니지만 !
hashable , equatable 은 무엇일까요 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Equatable
Equatable
프로토콜을 준수하는 타입은 비교연산자를 이용해 값을 비교할 수 있습니다.
즉, 타입끼리 비교 연산을 하기 위해서 구현해야 하는 프로토콜이 Equatable
입니다.
(기본 자료형(Int, String 등) 또한 Equatable
을 채택하고 있기 때문에 값의 비교가 가능한 것)
protocol Equatable {
static func == (lhs: Self, rhs: Self) -> [Bool]
}
해당 프로토콜에는 위와 같은 메서드가 정의 되어있는데, 이 메서드를 구현해주어야 값의 비교가 가능해집니다.
Hashable
Hasher로 해싱하여 Integer 해시 값(= hash value. 이걸 key로 값을 찾을 수 있음)을 가질 수 있는 타입입니다.
*값을 해싱하는 것: 해시함수에 넣는 것
(key를 해싱 함수에 넣으면 해싱 함수의 결과 값인 해시 주소 값 위치의 해시 테이블 공간에 Value를 저장)
즉, 해싱할 수 있도록 하기 위해서는Hashable
프로토콜을 준수해야 합니다.
Hashable
을 준수하는 타입은 Set
타입으로 사용되거나, Dictionary
의 key 로 사용될 수 있습니다.
(둘 다 해시 테이블을 사용하는 타입. 둘 다 중복을 허용하지 않는 타입이므로 Set의 요소와 Dictionary의 key는 모두 고유한 값)
protocol Hashable: Equatable {
var hashValue: Int { get }
func hash(into hasher: inout Hasher)
}
필요한 경우, 직접 hash(into:)
메서드를 구현해주어야 합니다.
Hashable이 Equatable을 채택하는 이유?
hashValue가 항상 unique하지 않기 때문입니다. 즉, hashValue가 같아서 해시 충돌이 일어날 가능성 때문입니다.
hashValue로만은 정확히 원하는 값을 찾을 수 없을 가능성이 있기 때문에, 추가로 값이 동일한지 확인하는 Equatable
이 필요한 것입니다. Equatable의 구현사항인 == 함수로 hashValue가 고유값인지 식별할 수 있습니다.
리뷰어:
@July911
버드:
@jane1choi
@suojae3
안녕하세요 July!☺️
세번째 PR 잘부탁드립니다
🔍 What is this PR?
은행창구 관리앱 STEP3 기능을 구현했습니다
✏️ PR Point
실제 은행 업무와 같이 업무별로 줄을 세워 처리
실제 은행 업무처리처럼 용건별로 손님들을 줄 세우는 방법을 사용했습니다!
현재 구현사항에서는 데이터 경쟁이 따로 없지만 큐를 actor로 선언해 Race Condition을 방지했습니다
async/await을 통한 동시성 처리
동시성 처리 방법에 있어 가장 간결하게 쓸 수 있는 키워드가
async let
이라 판단되어 사용했습니다!이때 총 작업 시간측정을 해야했기 때문에 비동기가 끝날 때까지 기다려주는
await
키워드를 사용했습니다!📮 질문
현재 async let 을 통해 별다른 세마포 처리없이 비동기 처리를 iOS에게 맡기고 있습니다.
Deposit 업무 처리를 두 개의 비동기로 돌리고 있기 때문에 프린트 로그는 구현사항에 맞추었으나
로그만 보았을 때는 스레드를 최대 4개만 쓸 거라 생각했는데 더 많은 스레드를 사용하고 있어서
어떻게 이런 현상이 발생하는건지 잘 모르겠습니다ㅠ