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 4] Jane, Sajae #98

Open
wants to merge 31 commits into
base: d_Sajae
Choose a base branch
from

Conversation

jane1choi
Copy link

@jane1choi jane1choi commented Feb 9, 2024

리뷰어:
@July911

버드:
@jane1choi
@suojae3

안녕하세요 July!
마지막 PR인 STEP 4 PR 올립니다!! 마감 기한을 넘겨버렸지만 흔쾌히 코드 봐주신다고 해주셔서 감사합니다!!
이슈가 좀 있는 상태인데... 같이 해결해봐요....ㅎ(디버깅의 신 July..짱..) 이번 PR도 잘 부탁드립니다 ☺️

참고로 Step3 PR이 열려있는 상태인데,
해당 PR에서의 변경사항은 Step3 브랜치와 무관하게 아예 다른 프로젝트 파일에서 작업한 것이고, main에서 뻗어나온 브랜치에서 작업해서 머지 순서 고려하지 않아주셔도 될 것 같습니다!! (그냥 편하게 머지해주시면 됩니다~!)

🔍 What is this PR?

은행창구 관리 앱 STEP 4 기능을 구현했습니다.

<구현 사항>
UI 구현하기 (Step 3의 은행을 UI 앱으로 전환합니다.)

✏️ PR Point

1. StackView 와 TableView를 이용한 화면 구성

스크린샷 2024-02-09 오후 1 18 31

StackViewTableView를 이용해 위와 같은 구조로 화면 UI 를 구성했습니다.
구현 요구사항에는 대기열 또한 StackView를 이용하도록 제시되어 있지만,
대기열이라는 기능 특성 상 같은 요소가 여러 개 존재하고, 요소의 삭제와 추가가 빈번히 이루어질 것을 고려하여 TableView를 이용했습니다.

2. 클로저를 이용한 인스턴스 초기화 (Then 라이브러리 참고)

클로저를 이용해 인스턴스 초기화 코드가 직관적이고 깔끔해질 수 있도록 했습니다.
코드는 Then 라이브러리를 참고해 구성했습니다.
Then 이라는 이름의 프로토콜을 생성 후 프로토콜 extension으로 클로저를 정의해두고 NSObject 의 모든 subclass에서 해당 코드를 사용할 수 있도록 했습니다.

protocol Then {}

extension Then where Self: AnyObject {
    
    @inlinable
    func then(_ block: (Self) throws -> Void) rethrows -> Self {
      try block(self)
      return self
    }
}

extension NSObject: Then {}

3. 아키텍처 - MVVM

MVVM 구조로 설계하여 View(해당 프로젝트에서 VC가 View의 역할 수행) 는 UI를 그리는 역할만 하고,
ViewModel은 비즈니스 로직을 담당하도록 하여 역할을 분리했습니다.
또한, ViewModel이 필요한 데이터를 요청하면 Repository가 데이터를 가져올 수 있도록 했습니다.
(DB가 있고 네트워크 작업을 한다면 Model에 해당하는 외부 데이터를 가져오는 역할을 수행하도록)

이 과정에서 라이브러리 없이 역할을 분리하여 코드를 구성하려니 클로저가 매우 많이 사용하게 되고,
데이터가 변경되는 시점을 체크해서 업데이트를 일일이 해주어야 한다는 단점이 있어서 didSet과 클로저를 활용해 Rx의 옵저버블 스트림 역할을 하는Observable 타입을 만들어두고 사용했습니다.

final class Observable<T> {

    private var listener: ((T) -> Void)?
    
    var value: T {
        didSet {
            self.listener?(value)
        }
    }

    init(_ value: T) {
        self.value = value
    }
    
    func subscribe(_ closure: @escaping (T) -> Void) {
        closure(value)
        self.listener = closure
    }
}

+ 뷰모델을 프로토콜을 활용해 Input-Output 구조로 정형화해서 깔끔하게 사용하려고 했는데,
기능 구현에도 문제가 있어서.. 현재는 ViewModel이라는 프로토콜만 존재합니다. 이슈가 해결되면 한 번 리팩토링 해보려고 합니다!
Input으로는 정말 이벤트나 상태값만 뷰모델로 넘겨주고 뷰모델은 이를 이용해 로직을 처리한 다음 Output를 뷰로 리턴해주어 이를 뷰에 바인딩하도록 UI를 업데이트 하여 뷰는 뷰모델에 대해 전혀 모르도록 명확히 역할 분리를 해주려고 했는데, Rx 없이 Input-Output 구조로 설계하려니 어렵더라구요..🥲 (디코로 언급해주셨듯이 지금은 뷰모델의 메서드나 값에 직접 접근하고 있는데,, 요걸 나중에 리팩토링 해보겠습니다..)

4. DispatchQueue와 DispatchSemaphore를 이용한 비동기 처리

DispatchQueue를 이용해 비동기 처리를 해주었고, 스레드 개수를 제한하여야 한다는 요구사항 구현을 위해 DispatchSemaphore를 이용했습니다.

📮 질문

DispatchSemaphore vs. serial Queue

명세에 스레드 개수를 대출 작업의 경우 1개, 예금 작업의 경우 2개로 제한해야 한다는 내용이 있어 세마포어를 이용했는데요!
코드를 좀 더 깔끔하게 작성하기 위해 두 작업의 처리 방식을 통일하여 세마포어를 두 개 정의해두고 각 작업에 해당하는 세마포어를 사용해 알맞은 스레드 개수를 조정해줄 수 있도록 했습니다.

그런데, 대출 작업의 경우 1개의 스레드만 사용해야 한다면 사실 굳이 세마포어를 사용하지 않고 serial queue를 이용해도 된다고 생각하는데, 어떤 것이 더 좋은 방법인지 고민되어 리뷰어님의 생각도 듣고 싶어 질문 남깁니다! (물론 두가지 방법 모두 장단점이 있겠지만요,,)

세마포어 관련 우려되는 문제

private func handleTask(of client: Client) {
        let semaphore = client.bankTask == .loan ? loanSemaphore : depositSemaphore
    
        semaphore.wait()
        DispatchQueue.global().async { [weak self] in
            guard let self = self else {return}
            Thread.sleep(forTimeInterval: client.bankTask.requiredTime)
            if self.workingClients.value.isEmpty { return }
            self.workingClients.value.removeFirst()
            semaphore.signal()
        }
    }

현재 위와 같이 대출, 예금 작업을 동시에 처리해주고 있는데요!
만약 작업중인 손님이 0명이 되어(= 더 이상 처리할 작업이 남지 않게 되어) if self.workingClients.value.isEmpty { return } 요 부분에서 return 으로 빠지게 될 경우 semaphore.wait() 으로 lock을 걸어놓고 signal 로 풀어주지 않게 되어 영원히 사용 가능한 스레드 개수가 늘어나지 않는 문제가 생기지 않을지..? 하는 생각이 들어서

if self.workingClients.value.isEmpty { 
    semaphore.signal()
    return 
}

요렇게 수정해주어야 할지 고민이 됩니다..!! 요부분도 같이 체크해주시면 감사하겠습니다!!!

jane1choi and others added 30 commits February 6, 2024 11:25
@@ -441,7 +549,8 @@
ASSETCATALOG_COMPILER_APPICON_NAME = AppIcon;
ASSETCATALOG_COMPILER_GLOBAL_ACCENT_COLOR_NAME = AccentColor;
CODE_SIGN_STYLE = Automatic;
INFOPLIST_FILE = BankManagerUIApp/Info.plist;
DEVELOPMENT_TEAM = VGM28F2FH7;
INFOPLIST_FILE = BankManagerUIApp/Resource/Info.plist;
Copy link

Choose a reason for hiding this comment

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

Resources 가 맞는거 같습니다.

path = App;
sourceTree = "<group>";
};
77F8FBAA2B7469630085F544 /* Util */ = {
Copy link

Choose a reason for hiding this comment

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

Util 복수로 바꿔주세요.

77F8FBB02B746A460085F544 /* Type */ = {
isa = PBXGroup;
children = (
08FE43F32B73972500D09BB7 /* Client.swift */,
Copy link

Choose a reason for hiding this comment

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

모델에 있어야할거같습니다.

77F8FBB82B74B67D0085F544 /* Model */ = {
isa = PBXGroup;
children = (
77F8FBB92B74B6A40085F544 /* BankRepository.swift */,
Copy link

Choose a reason for hiding this comment

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

모델..이라고 보는게 맞을까요 ? 여러분들이 생각하는 모델은 무엇인가요 ?


window = UIWindow(windowScene: windowScene)
window?.rootViewController = BankViewController(viewModel: BankViewModel())
window?.makeKeyAndVisible()
Copy link

Choose a reason for hiding this comment

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

무엇을 위해 호출해주어야 하나요 ?


// MARK: Properties
private let repository = BankRepository()
private let timer = BankTimer()
Copy link

Choose a reason for hiding this comment

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

init에서 받으면 좋을 것 같네요.

}

// MARK: Custom Methods
private func configureUI() {
Copy link

Choose a reason for hiding this comment

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

위치가 아래와 바뀐 것 같습니다 !


extension UIStackView {

/// 다수의 View를 StackView에 한꺼번에 추가하는 메서드
Copy link

Choose a reason for hiding this comment

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

의미있는 주석일까요 ?

extension UITableViewCell {

static var className: String {
NSStringFromClass(self.classForCoder()).components(separatedBy: ".").last ?? ""
Copy link

Choose a reason for hiding this comment

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


extension Then where Self: AnyObject {

@inlinable
Copy link

Choose a reason for hiding this comment

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

@inlinable 이 무엇이고 어떤 이점을 가지나요 ?

@@ -22,7 +22,7 @@ final class BankViewModel {
}

func fetchTimeString() {
timer.getTimeString = {
Copy link

Choose a reason for hiding this comment

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

timeString 이란 이름도 조금 모호한 것 같아요 !

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.

3 participants