-
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] Kyle, Effie #96
base: d_Kyle
Are you sure you want to change the base?
Conversation
- run script 추가 - lint rule 추가
- 은닉화
- LinkedList: tail 속성 추가 - Queue: front 및 rear 속성 추가 - LinkedList: getFirst() 메서드 -> first 연산 프로퍼티로 변경 - LinkedListTest/Queue: first 연산 프로퍼티 적용
- LinkedList & Queue: clear() 로직 추가 및 적용 - LinkedList: add() 메서드 불필요 로직 수정
- PR 에 업로드할 second-design
- BankTaskType > BankTask - TicketDispenser의 indicesByTask > ticketNumbers
- fatalError 호출 코드 제거 > throwing
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.
안녕하세요~ 두분 명절 연휴 직전까지 정말 고생많으셨습니다!
먼가 UI부분에서는 제가 코맨트를 많이 드릴 수 있을 줄 알았는데.. 사실 너무 잘해주셔서 머가 없습니다 ㅠㅠ
두분이 하신것중 ReusableView
가 가장 인상 깊었던것 같네요 저는!!
잘해주신점
- DiffableDataSource 사용 (Snapshot이 정말 강력하죠!)
- BankManager, ViewModel, VC간의 데이터 처리 및 Protocl 추상화
- Sempahore, GCD를 활용한 동시성 관리
질문에 대한 답변
사이드 이펙트에 따른 시행착오
플래그는 최대한 안쓰는게 좋으나, 필요하다면 사실 어쩔 수 없습니다
개발실력이 부족해서일수도 있고, 설계가 문제였을수도 있고, 아니면 그냥 쓸수밖에 없는 상황이었을수도 있습니다..!
말씀대로 상황마다 다르지만, 저는 개인적으로는 최대한 줄이면서 하려고 노력을 합니다.
첨부터 완벽하게 만들려고 하지말고, 방향만 크게잡고 덕지덕지 만들어논다음에, 해당 코드를 개선하는 식으로 해보면 현재 상황에서 최선까지 개선하실 수 있을거에요..!
버그를 잡다가 코드가 산으로 갈때는?
일단 할수 있는거 다했다고 가정한다면.. 할거 다해봐서 안되면 sos를 처야죠 ㅋㅋㅋ
주위 사람한테 물어볼것 같습니다 (지금 상황에서는 절허눈 동료나 리뷰어?)
다만 남의 코드를 보는거 자체가 쉬운일이 아니니, 코드를 깔끔히 작성하고 설명을 충분히 해줘야겠지만요.
이번 버그같은경우는 두분이 bp만 잘찍으셨으면 (물론 다 해보셨겠지만!) 충분히 잡을 수 있던 버그같아보여요..!
제가 처음에 한게, Clear후, 일단 VM에 있는 배열들이 제대로 바뀌나 확인을 해본거였고, 그래서 배열추가/삭제 하는곳에 bp를 찍어봤는데, 거기를 안탄다는게 바로 나왔거든요..!!
왜 안탈까? 생각을 해보면 semaphore가 의심될 수 밖에 없고, clear 시 버그가 생기니까 clear할때 호출되는 매서드를 봤더니, 배열을 다 날려버리는 로직이 있어서 찾아낼 수 있었습니다.
두분 정말 고생하셨습니다~~ !!
guard | ||
let index = self.workingList.firstIndex(where: { target in client == target }) | ||
else { return } |
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.
디코로도 남겼지만 이부분에 문제가 있네요~!
아니면 차라리 밑의 clearClients 부분에서
workingList를 싹 지워버릴때 역시 workingsemaphore로 감싸줘야 하는거 아닐까 싶습니다! (이게 더 맞는거같네요)
BankManagerUIApp/BankManagerUIApp/ViewModel/BankViewModel.swift
Outdated
Show resolved
Hide resolved
self.addClientButton.translatesAutoresizingMaskIntoConstraints = false | ||
self.clearButton.translatesAutoresizingMaskIntoConstraints = false | ||
self.buttonStackView.translatesAutoresizingMaskIntoConstraints = false | ||
self.timerView.translatesAutoresizingMaskIntoConstraints = false | ||
self.waitingListTableView.translatesAutoresizingMaskIntoConstraints = false | ||
self.workingListTableView.translatesAutoresizingMaskIntoConstraints = false | ||
self.listStackView.translatesAutoresizingMaskIntoConstraints = false | ||
self.containerView.translatesAutoresizingMaskIntoConstraints = false |
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.
아 이것도 항상 되게 고민되는 포인트인데 언급해주셨네요.. 저도 커스텀 뷰를 만들면 그냥 생성자만 호출해서 사용하고, 따로 뷰를 만들지 않으면 속성을 클로저 안에서 설정하는 식으로 구현을 하는 편인데요.
특히 이 translatesAutoresizingMaskIntoConstraints
속성이나, subview 의 제약이라던가, stack view의 subview를 추가하는 코드 같은 건 클로저에서 하는 게 맞는 걸까, 아니면 layout 설정하는 메서드에서 할까 고민이 되더라고요.
여기서는 커스텀 뷰가 많아서 나중에 한 번에 속성을 꺼주자 라고 결정했던 것 같은데. 최대한 일관성을 지키는 쪽으로 작성하는 연습을 하도록 하겠습니다!
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.
사람마다 다를수 있겠지만, 저같은경우는
addSubview(stackview도 마찬가지), autolayout은 따로 합니다.
translatesAutoresizingMaskIntoConstraints는 초기화시 합니다
lazy가 필요한 친구들은 굳이 초기화때 하지 않는것 같네요..!!
두분의 일관성만 있으면 될것 같습니다 :)
lazy var 같은경우는 GCD환경에서 좀 주의할 필요가 있겠죠..!!
BankManagerUIApp/BankManagerUIApp/ViewController/BankViewController.swift
Show resolved
Hide resolved
BankManagerUIApp/BankManagerUIApp/View/TableView/ClientListTableView.swift
Outdated
Show resolved
Hide resolved
BankManagerUIApp/BankManagerUIApp/View/TableView/ClientListTableView.swift
Show resolved
Hide resolved
self.window?.makeKeyAndVisible() | ||
} | ||
|
||
static func makeViewController() -> UIViewController { |
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으로 만든 이유가 있을까요?
그럴 필요가 없어보여서요!
let mirror = BankViewModel(bankManager: bankManager) | ||
bankManager.delegate = mirror | ||
|
||
let viewController = BankViewController(bankMirror: mirror) |
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.
좀 다른 이야기일 수 있지만(?) 저는 이번에 개인적으로 view controller - view model - domian 의 구조에서 발생하는 이벤트 흐름을 다음 단계 인터페이스 호출 + delegation 방식으로 구성하면서 순환 구조에서 deleagation의 치명적인 단점을 경험했다고 생각하는데요.
이 부분 코드를 보셔서 아시겠지만 주입의 방식으로 관련 객체의 사슬을 미리 구성하고 사용하다보니까 delegate 자체를 주입할 수 없다는 문제가 있더라고요. 그래서 구체 타입에서 모든 delegation 속성을 외부에 노출시킬 수 밖에 없고,
delegate를 설정해주기 위해 주입할 객체를 준비하는 과정에서 이 부분 코드처럼 특이한 순서로 객체를 생성하고 세팅해야 했습니다. 아마 대부분의 프로젝트에서는 reactivity를 활용할테니 이렇게 코드를 짜는 경우는 드물 것 같지만 🥲 작성하면서도 계속 이게 맞나... 싶은 코드였습니다.
써두고 보니 본문에서 언급했어야 하는 내용 같긴 한데, 뭔가 이번 구조를 짜면서 되게 인상 깊은 경험이었던 것 같아서 공유해두고 갑니다 ㅎㅎ
- 불필요하게 추가한 메서드를 제거하고 위임 코드를 바로 호출
- table view는 data source를 알 필요 없다.
두두(@FirstDo) 안녕하세요! 에피(@hyeffie), 카일(@Changhyun-Kyle)입니다.
이번 스텝에서는 지난 단계들에서 구현해 온 코드로 은행을 운영하는 화면을 그리는 요구사항을 구현했습니다.
지난 구현에서 사용했던 설계를 새로운 화면 요구사항에 그대로 반영하기 어려워, 화면을 그리기 위한 새로운 객체들이 필요했고 이 객체들과
View
, 그리고 모델이 조화를 이룰 수 있도록 구성하려 노력했습니다.폴더 구조
요구사항 구현
BankViewModel
의 도입앞서 언급했던 것처럼
STEP 3
에 반영했던 설계에서는 업무 종류 별로 고객 대기열과 은행원 대기열을 따로 관리하고 있어서, 이번 화면 요구사항을 그리기 위해서는 나누어진 고객 대기열을 합치는 과정이 필요했습니다. 화면에 표시할 데이터와 도메인의 주요 객체들에서 일어나는 변화를 맞추어 관리해야 했고, 이 역할을 위해BankViewController
를 미러링하고 있는BankViewModel
을 구현하게 되었습니다.이벤트 및 데이터 흐름
ViewController
,ViewModel
, 그리고 도메인을 구성하는 객체들 사이에 인터페이스를 구현해 책임을 위주로 통신하고,고민한 점
UITableView
와UITableViewDiffableDataSource
를 활용한 화면 요소 구현은행에 입장한 고객 대기열을 표현하기 위해
Queue
내지 목록 형태의view
가 필요했고,UIStackView
와UITableView
중 구현 비용이나 재활용 가능성 등을 고려해UITableView
와UITableViewDiffableDataSource
를 활용하기로 결정했습니다.ViewModel
로부터 전달받은 목록으로snapshot
을 생성해 적용하는 방식으로 이벤트를 화면에 반영하게 되었습니다!DispatchQueue.main() & DispatchQueue.global()
기존 구현한 로직은
BankViewController
에서startBank()
를 통해 은행 업무를 시작합니다. 해당 로직은 메인 스레드에서 동기로 처리하는 반면,startBank()
를 통해 실행되는BankManager
의runBank()
로직은global Queue
에서 비동기적으로 실행되고 작업이 완료될 때까지group.wait
을 통해 기다리게 됩니다. 이에 따라global queue
의 작업이 완료된 후 메인 스레드가 작동하며 모든 작업이 업무시간 이후에 한번에 처리되는 이슈 발생가 발생했습니다.따라서,
runBank()
메서드와addClients()
메서드를DispatchQueue.global()
로 비동기 처리를 해줌으로써 해당 로직이global Queue
에서 작동하여 은행업무의 순서대로 구현할 수 있었습니다.초기화 시 은행원의 조기 퇴근 이슈 🤬
기존 3-2 설계를 활용한 구조에서는 은행 업무 도중에 초기화 버튼을 누르면
banker
가bankerQueue
에enqueue
하지 못하고dequeue
된banker
가 로컬에만 저장되었다가 사라져서 아무리 고객을 추가해도 은행 업무가 진행되지 않는 이슈가 발생하였습니다. 위 문제는 해당 설계에서dequeue
된banker
가global Queue
에서 동작 후enqueue
되는 시점을 파악하기 어려워 도망간 은행원을enqueue
할 수 없었습니다.따라서, 기존 설계는 근본적으로 실행에 꼭 필요한
banker
가dequeue
되는 구조이기 때문에 개선이 어려울 것으로 판단하여 다시 3-1 설계를 활용하여 구조를 잡았습니다…하하…하지만,
SceneDelegate
에서banker
를 생성하여 은행 업무가 진행되는 설계인 설정하는 방법에서도 은행 업무 이후 초기화를 하면 타이머와 고객 업무는 진행되는데 업무중 대기열로 진입하지 않는 이슈가 발생했습니다…처음에는 구조 설계의 문제인 줄 알았지만, 수없이 디버깅을 해봐도 어떤 원인에서 이러한 현상이 발생하는 지 해결하지 못했습니다…
질문
사이드 이펙트에 따른 시행착오
BankManager 의 문제를 디버깅하기 위해서 객체의 상태만을 활용해 최대한 구현하려고 노력했지만, 만들 수 있는 조건으로도 부족해지자 다른 객체의 속성까지 추 하고 변경하면서 시도하게 되었습니다. 트러블슈팅에는 실패했는요. 혹시 지금 PR로 등록한 코드에서 어떤 부분이 문제인지 조언을 구하고 싶습니다. 😭
더불어 이럴 때는 상태(객체의 속성)와 조건을 최대한 줄이면서 구현을 하려 노력해야 하는지, 구현을 위해 상태를 충분히 활용해도 되는지..! 상황에 따라 많이 다르겠지만 어떤 방향이 좀 더 도움이 될지도 궁금합니다.
버그를 잡다가 코드가 산으로 갈 때는?
구현에 대한 질문은 아니지만, 마지막 스텝인만큼 두두의 조언을 얻고 싶은 부분이 있는데요..! 위에서 보셨다시피 이번에 콘솔과는 다른 UI에 대응하면서도 이전 구현을 활용하기 위해서 이틀을 꼬박 재설계와 디버깅으로 보냈던 것 같습니다(
에피와 함께하는 해커톤 시작
). 설계를 여러번 번복해도, 아무리 이런 저런 시도를 해도 버그를 잡기 어려울 때 두두는 어떤 방법을 사용하시는지, 설계를 적극적으로 뒤집는 것도 도움이 된다고 생각하시는지 궁금합니다!