은행 창구 매니저 [STEP2] Bard, Finnn #199

finnn1 · 2022-07-02T05:57:25Z

안녕하세요 우디! ☺️
STEP2 PR 보내드립니다!
이번에도 잘 부탁드립니다~!🙏

은행 창구 매니저 STEP2 PR

STEP2 UML

궁금한점 🤔

Protocol Oriented Programming 적용

POP 방식으로 처음 프로젝트를 진행해 보았는데, 저희가 제대로 된 POP 방식으로 진행을 한 것인지 확실치 않아서 여쭤봅니다..! 저희가 스스로 찾아봤는데 이해하기가 어려워서, POP에 대한 괜찮은 참고자료가 있다면 알려주시면 감사하겠습니다! 😢🙏

BankManager의 활용법

BankManager를 직역하자면 은행의 관리자로 보이는데 현재 은행 자체에서 고객을 받아서 은행의 직원Banker를 관리해야 하는 것인지, 아니면 은행의 관리자가 고객을 받아서 Banker들을 관리하는 것인지 의문이 듭니다.
일단, BankManager파일은 그대로 두고 Console이라는 타입을 만들어 Console이라는 타입에 콘솔앱을 시작해주는 메서드를 만들어 콘솔앱이 실행될 수 있도록 구현해보았습니다.

고민한점 🤔

Node 타입에 weak 적용

final class Node<T> {
    weak var next: Node<T>?
    weak var previous: Node<T>?
    var data: T
    
    init(data: T) {
        self.data = data
    }
}

처음에는 위와 같이 next 노드와 previous 노드를 둘 다 weak으로 설정해주었습니다.
하지만, 이렇게 할 경우 노르를 추가할 때마다 해당 노드를 Strong 하게 가리키는 노드가 없어서 중간에 있는 노드들이 deinitialize 되는 현상이 있었습니다. 고민 끝에 next 또는 previous 노드 둘 중 하나의 노트에만 weak으로 주는 것으로 해당 문제를 해결했습니다.

final class Node<T> {
    var next: Node<T>?
    weak var previous: Node<T>?
    var data: T
    
    init(data: T) {
        self.data = data
    }
}

- Agency 프로토콜 구현 - Customer 프로토콜 구현 - Worker 프로토콜 구현

Feat 2 protocol

bar-d · 2022-07-02T12:13:49Z

@Wody95 저희가 태그를 안달았네요...!

feat: main파일 내 콘솔 실행되도록 구현

Wody95 · 2022-07-04T14:34:09Z

BankManagerConsoleApp/BankManagerConsoleApp/Agency.swift

+//  Created by finnn, bard on 2022/07/01.
+//
+
+protocol Agency {


Agency - 대리점이라는 뜻으로 풀이되는데 은행 ~점 이렇게 이해하면 될까요?

Apple은 Protocol naming을 어떻게 하는지 알아보시면 더 좋을 것 같습니다.

다른 타입을 구현하게 된다면 Agency라는 프로토콜을 통해 Agency와 관련된 타입을 구현할 수 있다고 생각하여서 Agency라는 프로토콜을 채택하였습니다.
Bankable이라는 프로토콜 네이밍을 할 수도 있었지만, 해당 네이밍이 어색하다는 느낌이 들어 좀 더 범용적인 느낌의 Agency로 채택하였습니다

굳이 채택할 필요가 없다면 채택안해도 될 것 같습니다..!

Wody95

바드, 핀 STEP2 진행하시느라 수고 많으셨습니다 :)

바로 궁금하신 점, 고민했던 점 코멘트 드리겠습니다.

궁금한점

Protocol Oriented Programming 적용

- POP 방식으로 처음 프로젝트를 진행해 보았는데, 저희가 제대로 된 POP 방식으로 진행을 한 것인지 확실치 않아서 여쭤봅니다..! 저희가 스스로 찾아봤는데 이해하기가 어려워서, POP에 대한 괜찮은 참고자료가 있다면 알려주시면 감사하겠습니다! 😢🙏

먼저 Swift Language Guide - Protocol을 참고해보시면 좋을 것 같습니다.
Protocol의 목적과 사용 예제를 참고하시고 프로토콜 네이밍은 어떻게 하는지 보시면 POP에 대한 방향을 잡으실 수 있을 것 같습니다.

https://duwjdtn11.tistory.com/630
위 링크의 블로그도 읽어보시면 큰 도움이 될 것 같습니다.

BankManager의 활용법

이번 프로젝트의 핵심 경험에는 Queue라는 자료구조의 구현 및 활용과 타입 추상화 및 일반화 가 있습니다.
STEP2의 목표 중 하나인 은행과 고객은 어떤 역할을 하며, 서로에게 어떤 영향을 끼치는지는 여러분들이 정해야 합니다.
추상화와 일반화가 어렵다면 현실의 은행에서 은행원과 고객이 어떤 관계로 돌아가는지 생각해보시면 좋을 것 같습니다. 바드와 핀이 경험한 은행은 어떻게 돌아가고 있었나요?
++ 타입의 추상화와 일반화가 어렵다면 객체지향의 사실과 오해 라는 책을 한번 읽어보시는게 좋습니다. 저도 캠프생활을 하면서 스터디한 책인데요. 타입을 구현하는데 많은 도움이 된 책입니다.

고민한 점

Node 타입에 weak 적용

Strong Reference Cycle의 위험을 잘 풀어내셨습니다. 따로 드릴 코멘트가 없네요 👍

Wody95 · 2022-07-04T14:38:20Z

BankManagerConsoleApp/BankManagerConsoleApp/Agency.swift

+    associatedtype T: Equatable
+    var customerQueue: Queue<T> { get set }
+
+    mutating func generateEmployee(numberOfBankers: Int)


함수와 매개변수의 네이밍도 마찬가지입니다.

직원 생성~ 이라는 이름은 어색한 것 같습니다. (+ 실제로 프로토콜을 채택한 구조체에서 이 메서드를 구체적으로 구현하지 않는것을 확인했습니다. 사용하지 않는 메서드라면 삭제하는게 좋습니다)

만약 메소드를 사용하게 된다면 higherEmployee()라는 메서드가 더 명확한 메서드가 될 것 같네요!

해당 메소드 STEP3에서 사용하실거죠....?!

만약 STEP3에서도 사용되지 않는다면 삭제 부탁드립니다 :)

Wody95 · 2022-07-04T14:40:55Z

BankManagerConsoleApp/BankManagerConsoleApp/Agency.swift

+    mutating func openAgency()
+    mutating func closeAgency()


Agency라는 이름은 빼는게 맞다고 생각합니다. 메서드의 호출 앞에 누가 여는지 알 수 있으니깐요. Bank.open

넵 해당 메서드에 Agency 키워드는 삭제하였습니다!
감사합니다 🙏

Wody95 · 2022-07-04T14:43:20Z

BankManagerConsoleApp/BankManagerConsoleApp/Bank.swift

+    }
+
+    mutating func closeAgency() {
+        print("업무가 마감되었습니다. 오늘 업무를 처리한 고객은 총 \(customerCount ?? 0)명이며, 총 업무시간은 \(banker.totalWorkTime.formatDoubleToString())초입니다.")


옵셔널한 값에 대한 예외처리를 잘 하셨습니다. 👍👍👍👍👍

Nil-coalescing operator을 사용해보았습니다 ☺️🙏

Wody95 · 2022-07-04T14:46:29Z

BankManagerConsoleApp/BankManagerConsoleApp/BankCustomer.swift

+struct BankCustomer: Customer {
+    var id: Int
+
+    static func generateRandomCustomer(of range: ClosedRange<Int> = NameSpace.randomCustomerNumberRange) -> Queue<BankCustomer> {


고객이 고객 생성이라는 이름과 기능은 어색한 것 같습니다.
또한 고객이 Queue를 반환하여 은행� 대기열(Queue)에 값을 주입하는건 적절하지 않다고 생각하는데요. (Console에서)

핀과 바드의 생각은 어떠신가요?

고객을 생성한다는 이름으로 하지않고, 번호표를 뽑는 다는 의미가 더 자연스럽다고 생각되어 네이밍을 변경하였습니다!

Wody95 · 2022-07-04T14:50:12Z

BankManagerConsoleApp/BankManagerConsoleApp/Banker.swift

+import Foundation
+
+struct Banker: Worker {
+    var totalWorkTime: Double


은행원을 구현한 타입으로 보입니다.

현재 스탭에선 한 은행원이 한명의 고객의 업무만 진행하는걸로 보이는데 (n명의 은행원, n명의 고객)
totalWorkTime은 한 은행(Agency)의 총 업무시간을 갖고 있는걸로 보입니다.

제가 이해한게 맞을까요?

현재는 한명의 은행원만 은행에 근무하고 있기에 총 업무 시간은 곧 은행원의 업무 시간이라고 생각하여 totalWorkTime이라는 프로퍼티를 은행원 내부에 구현해 주었습니다.
스텝3에서는 은행에 넣어주기로 핀과 함께 리팩토링 하기로 결정한 후에 진행한 사항입니다.

Wody95 · 2022-07-04T14:51:25Z

BankManagerConsoleApp/BankManagerConsoleApp/Banker.swift

+
+struct Banker: Worker {
+    var totalWorkTime: Double
+    private var enqueueCount: Int


private한 enqueueCount는 현재 Banker에서만 정의되고, 초기화 과정에서만 사용하고 있습니다. 어떤 역할인가요?

현재 Customer가 가지고 있는 프로퍼티인 id와 동일한 일을 하는 프로퍼티인데 지우지를 못했습니다!

Wody95 · 2022-07-04T14:52:53Z

BankManagerConsoleApp/BankManagerConsoleApp/Banker.swift

+    mutating func startWork(of customer: BankCustomer) {
+        printStartWork(of: customer)
+        usleep(700000)
+        self.totalWorkTime += 0.7
+    }
+
+    mutating func finishWork(of customer: BankCustomer) {
+        printFinishWork(of: customer)
+    }


Work 이름은 제거하거나, 업무의 시작과 끝을 합쳐 하나의 메서드로 만들어도 괜찮아보입니다. 혹시 작업을 나눈 이유가 있을까요?

업무의 시작과 업무가 끝이 나는 것은 명백히 다른 일이라 생각하여 해당 메서드들을 나누어 주었습니다.
아니면 finishWork()라는 함수를 starkWork()내부 마지막에 넣어주어도 된다고 생각합니다.
굳이 나눠주지 않고 하나로 합쳐도 된다는 생각이 드네요!
근데 startWork(of customer: BankCustomer)가 start(of customer: BankCustomer)보다 더 의미가 부합하다고 보는데 어떻게 우디는 생각하시나요?

저는 두 이름 모두 괜찮다고 생각합니다 :)

Banker라는 타입의 역할이 다양하지 않는다면 start라는 이름의 의미도 쉽게 알 수 있겠네요!

Wody95 · 2022-07-04T14:54:53Z

BankManagerConsoleApp/BankManagerConsoleApp/Banker.swift

+    private func printStartWork(of customer: BankCustomer) {
+        print("\(customer.id)번 고객 업무 시작")
+    }
+
+    private func printFinishWork(of customer: BankCustomer) {
+        print("\(customer.id)번 고객 업무 완료")
+    }


작업의 결과물을 출력하는 코드는 work 메서드에 포함시켜도 될 것 같습니다.
(같은 매개변수를 사용하며 분리가 필요할 정도의 복잡한 기능이 아니라고 생각합니다)

또한 일을 시작하는 것과 일을 시작한다고 출력하는 것은 다르다고 생각하는데요
같은 매개변수를 사용하며 복잡한 기능은 아니더라도 나누어 주는 것이 맞다고 판단하였습니다...!

Wody95 · 2022-07-04T14:56:49Z

BankManagerConsoleApp/BankManagerConsoleApp/Customer.swift

+//  Created by finnn, bard on 2022/07/01.
+//
+
+protocol Customer: Equatable {


추상화를 위해 프로토콜을 사용하신 것 같은데, Equatable을 채택한 것 외에 어떠한 기능도 정의되지 않았습니다.

Customer 프로토콜의 경우, STEP3에서 각자 업무를 보도록 task 프로퍼티를 갖도록 해주려고 했었는데, STEP2에서는 해당 기능이 필요하지 않아서 일단 비어둔 상태였습니다. 이번에 STEP3를 대비해서 해당 프로퍼티 미리 추가해두었습니다 ☺️

Wody95 · 2022-07-04T14:58:10Z

BankManagerConsoleApp/BankManagerConsoleApp/Worker.swift

+//  Created by finnn, bard on 2022/07/01.
+//
+
+protocol Worker {


프로토콜 이름이 적절하지 않은 것 같습니다.
(만약 이름짓기가 어렵다면 코멘트 남겨주세요 😉)

프로퍼티 이름을 Workable로 변경했습니다! 🙏

Wody95 · 2022-07-04T15:07:57Z

앗! 이건 칭찬해야하는게 깜빡할 뻔 했다!!

⭐️ UML 작성하신 점 매우 좋았습니다! ⭐️

- Agency 프로토콜 내부에 있는 generateEmployee()함수 제거 - Agency 프로토콜 내부 customerQueue 프로퍼티 ticketBumberQueue로 네이밍 변경

Wody95 · 2022-07-05T13:31:35Z

대망의 비동기 프로그래밍입니다! STEP3에서 뵙겠습니다.

finnn1 and others added 16 commits July 1, 2022 16:59

feat: NameSpace 열거형 추가

f9b5eeb

feat: Protocol 폴더 생성 및 프로토콜 구현

c56dc4e

- Agency 프로토콜 구현 - Customer 프로토콜 구현 - Worker 프로토콜 구현

feat: BankCustomer 타입 정의

e949506

feat: Banker 타입 정의

39bc68c

feat: Bank 타입 정의

66cb5de

refactor: Banker 구조체 Worker 프로토콜 채택

7f77ad2

feat: main 파일 내 콘솔 실행 함수 구현

d0102df

refactor: 콘솔앱 리팩토링

0ec2b06

feat: Double Extension 구현

7b35694

style: 컨벤션 수정

e973418

refactor: LinkedList 및 Node 타입 리팩토링

3809e48

style: 불필요한 여백 제거

96e30d5

Merge pull request #3 from finnn1/feat-2-Protocol

7a0e1d2

Feat 2 protocol

docs: STEP1 Document 추가

ce90ab9

feat: Console 구조체 구현

595fc43

docs: README.md 수정

5eee972

bar-d added 2 commits July 4, 2022 22:27

feat: main파일 내 콘솔 실행되도록 구현

eebdc67

Merge pull request #4 from finnn1/hotfix-step2

e301bf3

feat: main파일 내 콘솔 실행되도록 구현

Wody95 reviewed Jul 4, 2022

View reviewed changes

finnn1 and others added 3 commits July 5, 2022 01:16

style: 메서드 네이밍 수정

db6e5d2

refactor: 불필요한 함수 제거 및 프로퍼티 네이밍 변경

42f2528

- Agency 프로토콜 내부에 있는 generateEmployee()함수 제거 - Agency 프로토콜 내부 customerQueue 프로퍼티 ticketBumberQueue로 네이밍 변경

refactor: 불필요한 프로퍼티 제거

7ace28a

finnn1 requested a review from Wody95 July 5, 2022 07:03

Wody95 approved these changes Jul 5, 2022

View reviewed changes

Wody95 merged commit 34ce035 into yagom-academy:ic_6_finnn Jul 5, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

은행 창구 매니저 [STEP2] Bard, Finnn #199

은행 창구 매니저 [STEP2] Bard, Finnn #199

finnn1 commented Jul 2, 2022 •

edited

Loading

bar-d commented Jul 2, 2022

Wody95 Jul 4, 2022 •

edited

Loading

bar-d Jul 4, 2022

bar-d Jul 4, 2022

Wody95 left a comment

Wody95 Jul 4, 2022

bar-d Jul 5, 2022

Wody95 Jul 5, 2022

Wody95 Jul 4, 2022

finnn1 Jul 5, 2022

Wody95 Jul 4, 2022

finnn1 Jul 5, 2022

Wody95 Jul 4, 2022

finnn1 Jul 5, 2022

Wody95 Jul 4, 2022

bar-d Jul 4, 2022

Wody95 Jul 4, 2022

bar-d Jul 5, 2022

Wody95 Jul 4, 2022

bar-d Jul 4, 2022

Wody95 Jul 5, 2022

Wody95 Jul 4, 2022

bar-d Jul 4, 2022

Wody95 Jul 4, 2022

finnn1 Jul 5, 2022

Wody95 Jul 4, 2022

finnn1 Jul 5, 2022

Wody95 commented Jul 4, 2022

Wody95 commented Jul 5, 2022

은행 창구 매니저 [STEP2] Bard, Finnn #199

은행 창구 매니저 [STEP2] Bard, Finnn #199

Conversation

finnn1 commented Jul 2, 2022 • edited Loading

은행 창구 매니저 STEP2 PR

STEP2 UML

궁금한점 🤔

Protocol Oriented Programming 적용

BankManager의 활용법

고민한점 🤔

Node 타입에 weak 적용

bar-d commented Jul 2, 2022

Wody95 Jul 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Wody95 left a comment

Choose a reason for hiding this comment

궁금한점

고민한 점

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Wody95 commented Jul 4, 2022

앗! 이건 칭찬해야하는게 깜빡할 뻔 했다!!

Wody95 commented Jul 5, 2022

finnn1 commented Jul 2, 2022 •

edited

Loading

Wody95 Jul 4, 2022 •

edited

Loading