-
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
은행창구 관리앱 [ Step 4 ] 랄라, 범 #53
base: 2_Rarla
Are you sure you want to change the base?
Conversation
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.
수고하셨습니다!
궁금한 부분이나 이해가 가지 않는부분이 있다면, 커멘트로 남겨주세요!
대기중과 업무중의 제약조건을 동일하게 걸었는데, 업무중 리스트에 좌우 스크롤이 되는 이슈가 발생했습니다. 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 { |
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.
configure로 시작하는 함수들이 반환값을 갖는게 어색합니다.
UI 객체를 반환하는 함수는 명사형이면 더 자연스러울 것 같아요
runningListScrollView.addSubview(runningListStackView) | ||
} | ||
|
||
func setupAutoLayout() { |
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.
autoLayout을 따로 함수처리한건 좋은데, 하나의 함수에 많은 코드가 있으면 가독성이 떨어지기 때문에, 객체나 기능에 따라 더 분리하면 좋을 것 같아요!
import UIKit | ||
|
||
|
||
protocol BankUIDelegate: AnyObject { |
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.
해당 프로토콜을 Tellers와 BankManager에서 모두 사용하고 있는데, 실제로 사용하는 부분은 나눠져 있는 부분이 아쉽습니다.
프로토콜을 더 세분화하면 좋을 것 같습니다.
bankManager.delegate = self | ||
bankManager.depositTellers.delegate = self | ||
bankManager.loanTellers.delegate = self |
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.
개인적으로 한 뎁스까지만 접근하는게 객체지향에 적합한 코드라고 생각하기 때문에 ViewController에서 두 뎁스를 통해서 접근하는 BankManager -> Teller -> delegate까지 접근하는 부분이 조금 아쉽습니다.
BankManager 안에서 로직을 모두 처리해서 BankManager가 ViewController에게 delegate에게 데이터 업데이트를 전달해서 뷸르 업데이트를 하는 구조면 좋겠습니다.
하지만 이 방법은 구조를 많이 바꿔야해서 일단 알아두시고 고민해보면 좋을 것 같습니다!
let _ = bankView.readyListStackView.arrangedSubviews.map { $0.removeFromSuperview()} | ||
let _ = bankView.runningListStackView.arrangedSubviews.map { $0.removeFromSuperview()} |
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.
반환값이 필요없다면 map 대신 forEach를 사용하면 좋을 것 같습니다!
} | ||
} | ||
|
||
private func timerStop() { |
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.
startTimer와 같이 stopTimer로 문법에 자연스럽게 수정하면 좋을 것 같습니다!
private lazy var runningTitleLabel: UILabel = configureTitleLabel(text: "업무중", backgroundColor: .systemBlue) | ||
|
||
private lazy var listStackView: UIStackView = { | ||
let view = configureHorizontalStackView() |
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.
위 lazy var에 클로저로 구현하는 방식과 함수를 혼합해서 쓰면 코드를 위 아래로 왔다갔다 하면서 읽어야해서 가독성이 떨어지는 것 같아요. 하나로 통일하면 좋을 것 같습니다!
은행창구 관리앱 [ Step 4 ]
안녕하세요~! @corykim0829
4번째 PR을 요청드리게 되었습니다!
팀원 : 랄라, 범� (+뉴준성의 도움을 많이 받았습니다)
구현 사항
고민한 점
궁금한 점
view.bounces = false
속성을 추가해 반동을 줄여 해결되는 것 처럼 보이긴하지만 정확한 원인이 무엇인지 이해하지 못했습니다. 어떤 부분이 문제인지 궁금합니다.2023-11-16.2.36.30.mov