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 ] 랄라, 범 #53

Open
wants to merge 6 commits into
base: 2_Rarla
Choose a base branch
from

Conversation

snowy-summer
Copy link

@snowy-summer snowy-summer commented Nov 16, 2023

은행창구 관리앱 [ Step 4 ]

안녕하세요~! @corykim0829
4번째 PR을 요청드리게 되었습니다!

팀원 : 랄라, 범� (+뉴준성의 도움을 많이 받았습니다)

구현 사항

  • Step 3의 은행을 UI 앱으로 전환합니다.
  • 이번 프로젝트의 UI구성은 ‘코드'만 사용하여 구현합니다.
  • 고객 정보를 표현할 커스텀뷰를 구현합니다.
    • 뷰에 포함될 정보 : 고객번호, 업무종류
  • 대기열은 스택뷰를 활용합니다.
    • 화면 전체를 여러개의 중첩된 스택뷰로 구성하는게 편리할 수 있습니다.
  • 화면 구성
    • 업무중인 고객 리스트, 대기중인 고객 리스트업무시간 타이머, 고객 10명 추가 버튼, 초기화 버튼
  • 고객 10명 추가 버튼을 통해 임의의 10명의 고객이 추가됩니다.
  • 초기화 버튼을 누르면 대기중인 고객과 타이머 모두 초기화됩니다.
  • 모든 업무가 끝나고 대기중인 고객이 없으면 업무시간 타이머는 멈춥니다.
  • 고객이 추가되어 다시 업무를 처리하면 타이머가 동작합니다.
    • 초기화 하기 전까지 업무시간은 초기화하지 않고 누적합니다.
고객 추가 초기화 고객 연속 추가
aaa bbb ccc

고민한 점

  • 콘솔로 돌아가던 코드를 UI앱으로 구현하기 위해 코드를 수정하지 않고 가능한 방법으로 Notification을 활용하거나 Delegate Pattern을 활용하면 되겠다고 생각했습니다.
  • 비동기 코드로 UI를 추가하는 작업은 main 큐에서 작업이 되어야 한다는 점을 학습했습니다.
  • Timer가 스크롤되면 화면이 멈추는 경우가 발생해 RunLoop를 학습 및 활용했습니다.
  • ViewController에 UI관련 코드가 많아 BankView를 파일로 분리 한 후 추가해주도록 작업했습니다.
  • BankView에서도 비슷한 UI를 생성하는 코드의 중복을 막기위해 함수로 분리해 작업했습니다.

궁금한 점

  • 대기중과 업무중의 제약조건을 동일하게 걸었는데, 업무중 리스트에 좌우 스크롤이 되는 이슈가 발생했습니다. view.bounces = false 속성을 추가해 반동을 줄여 해결되는 것 처럼 보이긴하지만 정확한 원인이 무엇인지 이해하지 못했습니다. 어떤 부분이 문제인지 궁금합니다.
  • 초기화와 고객 10명 추가를 반복적으로 누르는 경우 업무중에 예금이 처리될 때까지 대출이 처리되지 않거나 예금이 3개로 보이는 이슈가 있습니다. 충분히 고민해봤지만 어느 부분이 잘못되었는지 정확한 확인이 어려웠습니다. 추측하기로는 초기화버튼을 클릭 했을 때, queue를 clear해줘도 doTask 내 비동기로 돌린 부분이 멈추지 않아 semaphore를 차지하고 있어 이를 완료할 때까지 기다렸다가 실행되는 것이 아닐까 생각하고 있습니다. 이 추측이 맞는지 궁금합니다. 또한 아니라면 어떤 부분이 문제인지 알려주실 수 있을까요?
    • workItem으로 doTask의 비동기를 묶어 works라는 배열로 저장한 후 reset 할 때 works를 돌아 cancel()을 이용해 비동기를 멈춰보는 시도를 해봤습니다. 하지만 생각대로 멈춰지지 않았습니다.
2023-11-16.2.36.30.mov

Copy link

@corykim0829 corykim0829 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!

궁금한 부분이나 이해가 가지 않는부분이 있다면, 커멘트로 남겨주세요!

대기중과 업무중의 제약조건을 동일하게 걸었는데, 업무중 리스트에 좌우 스크롤이 되는 이슈가 발생했습니다. view.bounces = false 속성을 추가해 반동을 줄여 해결되는 것 처럼 보이긴하지만 정확한 원인이 무엇인지 이해하지 못했습니다. 어떤 부분이 문제인지 궁금합니다.

스크롤뷰는 스크롤 방향에 따라서 width나 height를 고정해줘야 단방향 스크롤로 고정할 수 있습니다. stackView가 많아서 후에 추가되는 업무중 스크롤뷰 내부 uiview width가 정확하게 절반으로 잡히지 않는 것 같습니다.

layoutIfNeeded()와 같은 autoLayout을 업데이트 해주는 함수를 사용해서 포인트를 잡아서 어디에서 width가 제대로 안잡히는지 확인할 수는 있습니다. 지금은 코드가 복잡해서 디버그하는데는 조금 시간이 걸릴 것 같기는 합니다.

scrollView의 alwaysBounceHorizontal 프로퍼티를 사용해서 특정 방향 bounce를 막을 수도 있으니 참고하세요!

초기화와 고객 10명 추가를 반복적으로 누르는 경우 업무중에 예금이 처리될 때까지 대출이 처리되지 않거나 예금이 3개로 보이는 이슈가 있습니다. 충분히 고민해봤지만 어느 부분이 잘못되었는지 정확한 확인이 어려웠습니다. 추측하기로는 초기화버튼을 클릭 했을 때, queue를 clear해줘도 doTask 내 비동기로 돌린 부분이 멈추지 않아 semaphore를 차지하고 있어 이를 완료할 때까지 기다렸다가 실행되는 것이 아닐까 생각하고 있습니다. 이 추측이 맞는지 궁금합니다. 또한 아니라면 어떤 부분이 문제인지 알려주실 수 있을까요?
workItem으로 doTask의 비동기를 묶어 works라는 배열로 저장한 후 reset 할 때 works를 돌아 cancel()을 이용해 비동기를 멈춰보는 시도를 해봤습니다. 하지만 생각대로 멈춰지지 않았습니다.

현재 가장 큰 문제점은 현재 두개의 Teller객체가 각자 바라보고 있는 queue만 초기화 하고 고객 10명 추가 버튼을 누를 때 다시 그 queue에 추가되기 때문에
UI상으로는 queue가 초기화된 것 처럼 보이지만, Teller는 계속해서 일을 하고있어서 해당 이슈가 발생하는 것 같습니다.
정확하게 Teller의 일을 중단 시키는 로직이 필요해 보입니다.
현재는 Teller가 Teller 내부의 group에서 나와야 일이 중단되는 로직인데 해당 로직을 점검해보면 좋을 것 같습니다.


lazy var runningListStackView: UIStackView = configureVerticalStackView()

func configureVerticalStackView() -> UIStackView {

Choose a reason for hiding this comment

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

configure로 시작하는 함수들이 반환값을 갖는게 어색합니다.
UI 객체를 반환하는 함수는 명사형이면 더 자연스러울 것 같아요

runningListScrollView.addSubview(runningListStackView)
}

func setupAutoLayout() {

Choose a reason for hiding this comment

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

autoLayout을 따로 함수처리한건 좋은데, 하나의 함수에 많은 코드가 있으면 가독성이 떨어지기 때문에, 객체나 기능에 따라 더 분리하면 좋을 것 같아요!

import UIKit


protocol BankUIDelegate: AnyObject {

Choose a reason for hiding this comment

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

해당 프로토콜을 Tellers와 BankManager에서 모두 사용하고 있는데, 실제로 사용하는 부분은 나눠져 있는 부분이 아쉽습니다.

프로토콜을 더 세분화하면 좋을 것 같습니다.

Comment on lines +21 to +23
bankManager.delegate = self
bankManager.depositTellers.delegate = self
bankManager.loanTellers.delegate = self

Choose a reason for hiding this comment

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

개인적으로 한 뎁스까지만 접근하는게 객체지향에 적합한 코드라고 생각하기 때문에 ViewController에서 두 뎁스를 통해서 접근하는 BankManager -> Teller -> delegate까지 접근하는 부분이 조금 아쉽습니다.

BankManager 안에서 로직을 모두 처리해서 BankManager가 ViewController에게 delegate에게 데이터 업데이트를 전달해서 뷸르 업데이트를 하는 구조면 좋겠습니다.

하지만 이 방법은 구조를 많이 바꿔야해서 일단 알아두시고 고민해보면 좋을 것 같습니다!

Comment on lines +113 to +114
let _ = bankView.readyListStackView.arrangedSubviews.map { $0.removeFromSuperview()}
let _ = bankView.runningListStackView.arrangedSubviews.map { $0.removeFromSuperview()}

Choose a reason for hiding this comment

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

반환값이 필요없다면 map 대신 forEach를 사용하면 좋을 것 같습니다!

}
}

private func timerStop() {

Choose a reason for hiding this comment

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

startTimer와 같이 stopTimer로 문법에 자연스럽게 수정하면 좋을 것 같습니다!

private lazy var runningTitleLabel: UILabel = configureTitleLabel(text: "업무중", backgroundColor: .systemBlue)

private lazy var listStackView: UIStackView = {
let view = configureHorizontalStackView()

Choose a reason for hiding this comment

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

위 lazy var에 클로저로 구현하는 방식과 함수를 혼합해서 쓰면 코드를 위 아래로 왔다갔다 하면서 읽어야해서 가독성이 떨어지는 것 같아요. 하나로 통일하면 좋을 것 같습니다!

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