연락처 관리 앱 [STEP 3] Hong, Effie #89
Conversation
- Int형 id 추가
| func makeExistingContact(from request: AddContact.UpdateContact.Request) throws -> Contact { | ||
| let id = request.id | ||
| let name = try validateName(request.name) | ||
| let age = try validateAge(request.age) | ||
| let phoneNumber = try validatePhoneNumber(request.phoneNumber) | ||
| return Contact(name: name, phoneNumber: phoneNumber, age: age) | ||
| return Contact(id: id, name: name, phoneNumber: phoneNumber, age: age) | ||
| } |
There was a problem hiding this comment.
편집 화면에서 입력된 내용을 Contact로 만들어 전달하는데요. 이때 입력된 content와 id를 분리하는 방식과 기존 id를 바탕으로 새로운 Contact 인스턴스를 만들어 전달하는 방식 사이에서 고민이 있었습니다. 라이언은 어떤 편이 더 낫다고 생각하시나요?
| enum ContactUnavailableConfiguration { | ||
| static let noContacts: UIContentUnavailableConfiguration = { | ||
| var config = UIContentUnavailableConfiguration.empty() | ||
| config.image = UIImage(systemName: "person.crop.circle") | ||
| config.text = "저장된 연락처 없음" | ||
| config.secondaryText = "저장된 연락처 목록이 여기 표시됩니다." | ||
| return config | ||
| }() | ||
|
|
||
| static let noSearchingResults: UIContentUnavailableConfiguration = { | ||
| var config = UIContentUnavailableConfiguration.search() | ||
| config.text = "검색 결과 없음" | ||
| config.secondaryText = "조건과 일치하는 검색 결과가 없습니다." | ||
| return config | ||
| }() | ||
| } |
There was a problem hiding this comment.
여기 생성해둔 객체들은 전역에서 접근이 가능한데요. 화면 내부에서 접근하도록 만드는 방법이 나았을지 궁금합니다.
There was a problem hiding this comment.
이런 식으로 접근할 때마다 생성하는 방식도 있을 것 같습니다. 어느 편을 더 선호하시나요?
enum Configuration {
case noContacts
case noSearchingResults
private var text: String {
switch self {
case .noContacts:
return "저장된 연락처 없음"
case .noSearchingResults:
return "검색 결과 없음"
}
}
private var secondaryText: String {
switch self {
case .noContacts:
return "저장된 연락처 목록이 여기 표시됩니다."
case .noSearchingResults:
return "조건과 일치하는 검색 결과가 없습니다."
}
}
private var image: UIImage? {
switch self {
case .noContacts:
return UIImage(systemName: "person.crop.circle")
case .noSearchingResults:
return nil
}
}
var configuration: UIContentUnavailableConfiguration {
switch self {
case .noContacts:
return UIContentUnavailableConfiguration.empty()
case .noSearchingResults:
return UIContentUnavailableConfiguration.search()
}
}
}| func requestContacts() throws -> [Contact] { | ||
| return self.contactList.getContacts() | ||
| let contacts = self.contactList.getContacts() | ||
| guard contacts.isEmpty == false else { throw ContactRepositoryError.noContacts } | ||
| return contacts | ||
| } |
There was a problem hiding this comment.
연락처 목록이 비어 있는 상태에 대응하기 위해 에러를 던지도록 구현을 수정했습니다!
| private func match(_ query: String, to contact: Contact) -> Bool { | ||
| // 이름에 쿼리가 포함되는지 확인 | ||
| var result = contact.name.localizedCaseInsensitiveContains(query) | ||
|
|
||
| // 숫자로 바꿀 수 있는 쿼리라면 나이와 같은지 확인 | ||
| if let number = Int(query) { | ||
| result = result || (contact.age == number) | ||
| } | ||
|
|
||
| // 하이픈을 제외한 전화번호와 일치하는 부분이 있는지 | ||
| let purePhoneNumberString = contact.phoneNumber.filter { ch in ch.isNumber } | ||
| result = result || (purePhoneNumberString.localizedStandardContains(query)) | ||
|
|
||
| return result | ||
| } |
There was a problem hiding this comment.
이 코드를 리뷰하는 시점에서 보니 repository의 역할과 어울리지 않는 듯한 느낌이 있네요.
개발하는 시점에서는 네트워킹으로 검색 결과를 요청하는 것처럼 검색을 지원하는 소스가 있는 상황에서는 이런 구현이 어디로 가야할지 고민되었던 것 같습니다.
| enum AddContact { | ||
| struct Request { | ||
| let name: String | ||
| let age: String | ||
| let phoneNumber: String | ||
| enum CreatContact { | ||
| struct Request { | ||
| let name: String | ||
| let age: String | ||
| let phoneNumber: String | ||
| } | ||
| } | ||
|
|
||
| enum FetchContact { | ||
| struct Request { | ||
| let id: Int | ||
| } | ||
| } | ||
|
|
||
| enum UpdateContact { | ||
| struct Request { | ||
| let id: Int | ||
| let name: String | ||
| let age: String | ||
| let phoneNumber: String | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
하나의 화면을 재사용하다보니 데이터를 전달하기 위해 선언한 타입도 추가될 수 밖에 없었습니다. 그래서 네임스페이스를 한 단계 더 들여 구현하게 되었어요. 이렇게 재사용하면서 경험했던 문제점은 두 가지였는데요.
- 나쁜 가독성
추가 화면을 위해 만들었던 AddContact와 관련된 계층을 모두 재사용하다보니 업데이트를 위한 기능에서도 같은 이름을 사용할 수 밖에 없게 되었습니다. 코드를 작성하는 중간 중간 제 스스로도 헷갈릴 정도라 가독성이 굉장히 나빠진다고 생각했던 것 같아요.
There was a problem hiding this comment.
- 한 객체에 책임이 밀집되는 현상
지금은 Model, ViewController, UseCase, Coordinator 을 공유하고 있어서 같은 기능을 위한 구현이 하나의 객체에 모두 구현되어 있는 상태입니다. 인터페이스로 분리하려다 보니 어떤 계층은 추상화되어 있고, 어떤 계층은 되어 있지 않아서 쉽게 분리되기 어렵기도 했고요.
지금에서 use case를 분리하고, view controller를 제네릭으로 구현을 공유하도록 해야 할까요? 아니면 비용이 들고 코드가 중복되더라도 각 계층을 위한 다른 타입을 만드는 편이 나을까요?
이렇게 같은 화면을 재사용하는 상황에서 사용할 수 있는지 좋은 솔루션이 있는지 궁금합니다....!
| private var listIsEmpty: ListState = .noProblem { | ||
| didSet { | ||
| setNeedsUpdateContentUnavailableConfiguration() | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
목록의 상태에 따라 Unavailable 상태를 view에도 반영하도록 업데이트 인터페이스를 호출합니다.
| private func setSearchController() { | ||
| self.searchController = UISearchController(searchResultsController: nil) | ||
| self.searchController?.searchResultsUpdater = self | ||
| navigationItem.searchController = searchController | ||
| } | ||
|
|
There was a problem hiding this comment.
검색을 위한 search bar와 결과를 보여주는 view에 대한 display 를 관리하는 UISearchController를 설정하는 코드입니다. 현재 pull을 요청한 코드에는 현재 목록에 그대로 결과를 표시하도록 self를 사용하고 있습니다.
| extension ListContactViewController: UITableViewDelegate { | ||
| func tableView( | ||
| _ tableView: UITableView, | ||
| trailingSwipeActionsConfigurationForRowAt indexPath: IndexPath | ||
| ) -> UISwipeActionsConfiguration? { | ||
| guard let listItem = contactListDataSource.itemIdentifier(for: indexPath) else { return nil } | ||
| switch listItem { | ||
| case .contact(let contact): | ||
| let deleteAction = UIContextualAction(style: .destructive, title: "Delete") { _, _, _ in | ||
| self.listContactUseCase?.deleteContact(contactID: contact.id) | ||
| } | ||
| return UISwipeActionsConfiguration(actions: [deleteAction]) | ||
| } | ||
| } |
There was a problem hiding this comment.
테이블 뷰 셀 선택이 트리거가 되는 편집 기능과 셀 스와이프가 트리거가 되는 삭제 기능 모두 indexPath를 활용해 itemIdentifier를 구하고, item인 Contact의 id에 접근해 use case 의 기능을 호출하도록 구현했습니다.
| extension ListContactViewController: UISearchResultsUpdating { | ||
| func updateSearchResults(for searchController: UISearchController) { | ||
| guard let query = searchController.searchBar.text, query.isEmpty == false else { | ||
| self.listContactUseCase?.fetchAllContacts() | ||
| return | ||
| } | ||
| self.listContactUseCase?.searchContact(with: query) | ||
| } | ||
| } |
There was a problem hiding this comment.
현재 pull을 요청한 코드에는 현재 목록에 그대로 결과를 표시하도록 self를 사용하고 있고, 업데이트를 반영하는 UISearchResultsUpdating 역시 ListContactViewController가 구현하고 있는데요. 만약 다른 뷰 구성에 결과를 표시하도록 하고 싶다면 관련 로직을 어디에 두어야 할지에 대해 고민이 되어서 각 케이스를 구현해보았습니다. 라이언은 UISearchResultsUpdating을 어디에서 제공하는 게 더 적절하다고 생각하시나요?
| private let contactID: Int? | ||
|
|
There was a problem hiding this comment.
view controller 에 contactID를 전달하기 위해 Coordinator도 contactID를 전달받아 속성으로 유지하게 됩니다.
| guard let listItem = contactListDataSource.itemIdentifier(for: indexPath) else { return nil } | ||
| switch listItem { | ||
| case .contact(let contact): | ||
| let deleteAction = UIContextualAction(style: .destructive, title: "Delete") { _, _, _ in | ||
| self.listContactUseCase?.deleteContact(contactID: contact.id) | ||
| } | ||
| return UISwipeActionsConfiguration(actions: [deleteAction]) |
There was a problem hiding this comment.
인덱스 패스에 대한 데이터 소스의 아이템을 가져와 deleteContact메서드를 호출해 연락처를 삭제하는 동작을 만들고 스와이프 동작 구현에 넣어줘 데이터를 삭제하도록했습니다.
라이언(@ryan-son) 안녕하세요. 늦은 PR로 돌아온 에피(@hyeffie), 홍(@ujhong7)입니다!
이번 스텝에서는 제시된 요구사항 중 연락처 삭제, 연락처 검색, 연락처 상세 보기, 연락처 편집 기능을 선택해 구현했습니다.
searchResultsController로 다른 ViewController를 사용했을 때 검색을 트리거하고 결과 반영하는 로직을 어디에 위치시킬지에 대한 고민이 있었습니다.UIContentUnavailableConfiguration을 활용해 상태에 대한 정보를 표시하도록 구현했습니다.올려둔 코드에 대한 자세한 설명은 코멘트를 통해 작성하도록 하겠습니다. 이번 리뷰도 잘 부탁드리겠습니다!