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 3] Hong, Effie #89

Open
wants to merge 17 commits into
base: d_Effie
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions ContactManager/ContactManager.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
F42BF4772B54CA9C0067C8E8 /* ContactValidationError.swift in Sources */ = {isa = PBXBuildFile; fileRef = F42BF4762B54CA9C0067C8E8 /* ContactValidationError.swift */; };
F42BF4792B54CB6C0067C8E8 /* AddContactError.swift in Sources */ = {isa = PBXBuildFile; fileRef = F42BF4782B54CB6C0067C8E8 /* AddContactError.swift */; };
F42BF47B2B54CC620067C8E8 /* AddContact.swift in Sources */ = {isa = PBXBuildFile; fileRef = F42BF47A2B54CC620067C8E8 /* AddContact.swift */; };
F44184BE2B594C3D0011CB5B /* ContactUnavailableConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = F44184BD2B594C3D0011CB5B /* ContactUnavailableConfiguration.swift */; };
F456EA732B49511C001BE636 /* Contact.swift in Sources */ = {isa = PBXBuildFile; fileRef = F456EA722B49511C001BE636 /* Contact.swift */; };
F456EA762B4951E9001BE636 /* ContactList.swift in Sources */ = {isa = PBXBuildFile; fileRef = F456EA752B4951E9001BE636 /* ContactList.swift */; };
F456EA7A2B4954E4001BE636 /* ListContactUseCase.swift in Sources */ = {isa = PBXBuildFile; fileRef = F456EA792B4954E4001BE636 /* ListContactUseCase.swift */; };
Expand Down Expand Up @@ -49,6 +50,7 @@
F42BF4762B54CA9C0067C8E8 /* ContactValidationError.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ContactValidationError.swift; sourceTree = "<group>"; };
F42BF4782B54CB6C0067C8E8 /* AddContactError.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AddContactError.swift; sourceTree = "<group>"; };
F42BF47A2B54CC620067C8E8 /* AddContact.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AddContact.swift; sourceTree = "<group>"; };
F44184BD2B594C3D0011CB5B /* ContactUnavailableConfiguration.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ContactUnavailableConfiguration.swift; sourceTree = "<group>"; };
F456EA722B49511C001BE636 /* Contact.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Contact.swift; sourceTree = "<group>"; };
F456EA752B4951E9001BE636 /* ContactList.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ContactList.swift; sourceTree = "<group>"; };
F456EA792B4954E4001BE636 /* ListContactUseCase.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ListContactUseCase.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -137,6 +139,7 @@
F4B1787B2B539320005B8E78 /* Formatter.swift */,
F4B1788A2B53F0B1005B8E78 /* ContactMakable.swift */,
F42BF4762B54CA9C0067C8E8 /* ContactValidationError.swift */,
F44184BD2B594C3D0011CB5B /* ContactUnavailableConfiguration.swift */,
);
path = Common;
sourceTree = "<group>";
Expand Down Expand Up @@ -392,6 +395,7 @@
F456EA902B497E16001BE636 /* ContactRepository.swift in Sources */,
F4B178892B53C095005B8E78 /* AddContactCoordinator.swift in Sources */,
F456EA8A2B495C14001BE636 /* ContactListDataSource.swift in Sources */,
F44184BE2B594C3D0011CB5B /* ContactUnavailableConfiguration.swift in Sources */,
F4B178702B529351005B8E78 /* AddContactViewController.swift in Sources */,
F456EA762B4951E9001BE636 /* ContactList.swift in Sources */,
F456EA8C2B495E13001BE636 /* ReusableCell.swift in Sources */,
Expand Down
22 changes: 19 additions & 3 deletions ContactManager/ContactManager/Common/ContactMakable.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,31 @@
//

protocol ContactMakable {
func makeContact(from request: AddContact.Request) throws -> Contact
func makeContact(from request: AddContact.CreatContact.Request) throws -> Contact
func makeExistingContact(from request: AddContact.UpdateContact.Request) throws -> Contact
}

import Foundation

struct ContactFactory: ContactMakable {
func makeContact(from request: AddContact.Request) throws -> Contact {
func makeContact(from request: AddContact.CreatContact.Request) throws -> Contact {
let id = makeID()
let name = try validateName(request.name)
let age = try validateAge(request.age)
let phoneNumber = try validatePhoneNumber(request.phoneNumber)
return Contact(id: id, name: name, phoneNumber: phoneNumber, age: age)
}

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)
}
Comment on lines +24 to +30
Copy link
Author

Choose a reason for hiding this comment

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

편집 화면에서 입력된 내용을 Contact로 만들어 전달하는데요. 이때 입력된 content와 id를 분리하는 방식과 기존 id를 바탕으로 새로운 Contact 인스턴스를 만들어 전달하는 방식 사이에서 고민이 있었습니다. 라이언은 어떤 편이 더 낫다고 생각하시나요?


private func makeID() -> Int {
return UUID().hashValue
}

private func validateName(_ name: String) throws -> String {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
//
// ContactUnavailableConfiguration.swift
// ContactManager
//
// Created by Effie on 1/18/24.
//

import UIKit

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
}()
}
Comment on lines +10 to +25
Copy link
Author

Choose a reason for hiding this comment

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

여기 생성해둔 객체들은 전역에서 접근이 가능한데요. 화면 내부에서 접근하도록 만드는 방법이 나았을지 궁금합니다.

Copy link
Author

Choose a reason for hiding this comment

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

이런 식으로 접근할 때마다 생성하는 방식도 있을 것 같습니다. 어느 편을 더 선호하시나요?

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()
        }
    }
}

64 changes: 63 additions & 1 deletion ContactManager/ContactManager/Data/ContactRepository.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,17 @@
import Foundation

protocol ContactRepository {
func requestContact(id: Int) throws -> Contact

func requestContacts() throws -> [Contact]

func addContact(_ newContact: Contact) throws

func removeContact(contactID: Int) throws

func searchContact(with queries: [String]) throws -> [Contact]

func updateContact(with updatedContact: Contact) throws
}

struct ContactRepositoryImpl: ContactRepository {
Expand All @@ -28,13 +36,67 @@ struct ContactRepositoryImpl: ContactRepository {
self.fileProvider = fileProvider
}

func requestContact(id: Int) throws -> Contact {
do {
return try self.contactList.getContact(id: id)
} catch ContactListError.invalidID {
throw ContactRepositoryError.notFound
}
}

func requestContacts() throws -> [Contact] {
return self.contactList.getContacts()
let contacts = self.contactList.getContacts()
guard contacts.isEmpty == false else { throw ContactRepositoryError.noContacts }
return contacts
}
Comment on lines 47 to 51
Copy link
Author

Choose a reason for hiding this comment

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

연락처 목록이 비어 있는 상태에 대응하기 위해 에러를 던지도록 구현을 수정했습니다!


func addContact(_ newContact: Contact) throws {
self.contactList.addContact(newContact)
}

func removeContact(contactID: Int) throws {
do {
try self.contactList.deleteContact(contactID: contactID)
} catch ContactListError.invalidIndex {
throw ContactRepositoryError.cannotRemove
} catch {
throw error
}
}

func searchContact(with queries: [String]) throws -> [Contact] {
let contacts = self.contactList.getContacts()
var matches = contacts
for query in queries {
matches = matches.filter { contact in self.match(query, to: contact) }
}
guard matches.isEmpty == false else { throw ContactRepositoryError.noSearchingResult }
return matches
}

func updateContact(with updatedContact: Contact) throws {
do {
try self.contactList.updateContact(with: updatedContact)
} catch {
throw ContactRepositoryError.cannotUpdate
}
}

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
}
Comment on lines +85 to +99
Copy link
Author

Choose a reason for hiding this comment

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

이 코드를 리뷰하는 시점에서 보니 repository의 역할과 어울리지 않는 듯한 느낌이 있네요.
개발하는 시점에서는 네트워킹으로 검색 결과를 요청하는 것처럼 검색을 지원하는 소스가 있는 상황에서는 이런 구현이 어디로 가야할지 고민되었던 것 같습니다.

}

extension ContactRepositoryImpl {
Expand Down
26 changes: 26 additions & 0 deletions ContactManager/ContactManager/Data/ContactRepositoryError.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,29 @@ enum ContactRepositoryError: LocalizedError {
case notFoundAtBundle
case cannotDecode

case noContacts
case cannotRemove
case noSearchingResult
case notFound
case cannotUpdate

var errorDescription: String? {
var description: String = "\(String(describing: Self.self)).\(String(describing: self)): "
switch self {
case .notFoundAtBundle:
description += "요청된 파일을 번들에서 찾을 수 없음"
case .cannotDecode:
description += "요청된 타입으로 디코딩 실패"
case .cannotRemove:
description += "인덱스 문제로 삭제 실패"
case .noContacts:
description += "연락처 데이터 없음"
case .noSearchingResult:
description += "조건에 맞는 연락처 없음"
case .notFound:
description += "전달한 ID의 연락처 없음"
case .cannotUpdate:
description += "ID 문제로 업데이트 실패"
}
return description
}
Expand All @@ -30,6 +46,16 @@ extension ContactRepositoryError: AlertableError {
return .init(body: "표시할 연락처가 없어요.")
case .cannotDecode:
return .init(body: "데이터를 알맞은 형태로 바꿔줄 수 없어요.")
case .cannotRemove:
return .init(body: "선택한 연락처를 삭제할 수 없어요.")
case .noContacts:
return .init(body: "저장된 연락처가 없어요.")
case .noSearchingResult:
return .init(body: "검색 결과가 없어요.")
case .notFound:
return .init(body: "선택한 연락처가 존재하지 않아요.")
case .cannotUpdate:
return .init(body: "편집한 내용을 저장할 수 없어요.")
}
}
}
25 changes: 21 additions & 4 deletions ContactManager/ContactManager/Domain/AddContact/AddContact.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,26 @@
//

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
}
}
}
Comment on lines 8 to 31
Copy link
Author

Choose a reason for hiding this comment

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

하나의 화면을 재사용하다보니 데이터를 전달하기 위해 선언한 타입도 추가될 수 밖에 없었습니다. 그래서 네임스페이스를 한 단계 더 들여 구현하게 되었어요. 이렇게 재사용하면서 경험했던 문제점은 두 가지였는데요.

  1. 나쁜 가독성
    추가 화면을 위해 만들었던 AddContact와 관련된 계층을 모두 재사용하다보니 업데이트를 위한 기능에서도 같은 이름을 사용할 수 밖에 없게 되었습니다. 코드를 작성하는 중간 중간 제 스스로도 헷갈릴 정도라 가독성이 굉장히 나빠진다고 생각했던 것 같아요.

Copy link
Author

Choose a reason for hiding this comment

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

  1. 한 객체에 책임이 밀집되는 현상
    지금은 Model, ViewController, UseCase, Coordinator 을 공유하고 있어서 같은 기능을 위한 구현이 하나의 객체에 모두 구현되어 있는 상태입니다. 인터페이스로 분리하려다 보니 어떤 계층은 추상화되어 있고, 어떤 계층은 되어 있지 않아서 쉽게 분리되기 어렵기도 했고요.
    지금에서 use case를 분리하고, view controller를 제네릭으로 구현을 공유하도록 해야 할까요? 아니면 비용이 들고 코드가 중복되더라도 각 계층을 위한 다른 타입을 만드는 편이 나을까요?

이렇게 같은 화면을 재사용하는 상황에서 사용할 수 있는지 좋은 솔루션이 있는지 궁금합니다....!

Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,17 @@ struct AddContactUseCase {
self.factory = factory
}

func saveNewContact(request: AddContact.Request) {
func fetchContact(request: AddContact.FetchContact.Request) {
do {
let id = request.id
let contact = try self.repository.requestContact(id: id)
presenter?.presentFetchContact(result: .success(contact))
} catch {
presenter?.presentFetchContact(result: .failure(error))
}
}

func saveNewContact(request: AddContact.CreatContact.Request) {
do {
let contact = try factory.makeContact(from: request)
try repository.addContact(contact)
Expand All @@ -30,7 +40,17 @@ struct AddContactUseCase {
}
}

func confirmCancel(request: AddContact.Request) {
func updateNewContact(request: AddContact.UpdateContact.Request) {
do {
let contact = try factory.makeExistingContact(from: request)
try repository.updateContact(with: contact)
presenter?.presentUpdateContact(result: .success(()))
} catch {
presenter?.presentUpdateContact(result: .failure(error))
}
}

func confirmCancel(request: AddContact.CreatContact.Request) {
do {
try confirmIfCancellable(request: request)
presenter?.presentCancelConfirmation(result: .success(()))
Expand All @@ -39,7 +59,7 @@ struct AddContactUseCase {
}
}

private func confirmIfCancellable(request: AddContact.Request) throws {
private func confirmIfCancellable(request: AddContact.CreatContact.Request) throws {
guard request.name.isEmpty,
request.age.isEmpty,
request.phoneNumber.isEmpty else {
Expand All @@ -51,6 +71,8 @@ struct AddContactUseCase {
import Foundation

protocol AddContactPresentable: NSObjectProtocol {
func presentFetchContact(result: Result<Contact, Error>)
func presentAddContact(result: Result<Void, Error>)
func presentCancelConfirmation(result: Result<Void, Error>)
func presentUpdateContact(result: Result<Void, Error>)
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,33 @@ struct ListContactUseCase {
presenter?.presentListContact(result: .failure(error))
}
}

func deleteContact(contactID: Int) {
do {
try repository.removeContact(contactID: contactID)
presenter?.presentDeleteContact(result: .success(()))
} catch {
presenter?.presentDeleteContact(result: .failure(error))
}
}

func searchContact(with query: String) {
let trimmedQuery = query.trimmingCharacters(in: .whitespacesAndNewlines)
let queries = trimmedQuery.components(separatedBy: .whitespacesAndNewlines)
do {
let matchingContacts = try repository.searchContact(with: queries)
let successInfo = ListContact.SuccessInfo(contacts: matchingContacts)
presenter?.presentSearchContact(result: .success(successInfo))
} catch {
presenter?.presentSearchContact(result: .failure(error))
}
}
}

import Foundation

protocol ListContactPresentable: NSObjectProtocol {
func presentListContact(result: Result<ListContact.SuccessInfo, Error>)
func presentDeleteContact(result: Result<Void, Error>)
func presentSearchContact(result: Result<ListContact.SuccessInfo, Error>)
}
2 changes: 2 additions & 0 deletions ContactManager/ContactManager/Domain/Model/Contact.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
//

struct Contact: Hashable, Decodable {
let id: Int

Comment on lines 8 to +10
Copy link
Author

Choose a reason for hiding this comment

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

편집 기능을 구현하면서 view 차원의 index보다는 객체 고유값을 사용하는 편이 나을 것 같다고 판단해 id 속성을 추가하게 되었습니다. hash value를 id로 계산하면 diffable data source에서 hash의 diff를 인식하지 못하는 문제가 생겨서 전체 속성의 값으로 hash를 구성하는 방식을 유지했습니다.

let name: String

let phoneNumber: String
Expand Down
22 changes: 16 additions & 6 deletions ContactManager/ContactManager/Domain/Model/ContactList.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,13 @@ final class ContactList {
self.contacts = contacts
}

func getContact(id: Int) throws -> Contact {
guard let contact = self.contacts.first(where: { contact in contact.id == id }) else {
throw ContactListError.invalidID
}
return contact
}

func getContacts() -> [Contact] {
return self.contacts
}
Expand All @@ -26,19 +33,22 @@ final class ContactList {
self.contacts.insert(newContact, at: 0)
}

func deleteContact(at index: Index) throws {
guard validateIndex(index) else { throw ContactListError.invalidIndex }
func deleteContact(contactID: Int) throws {
let index = try getIndexofContact(id: contactID)
self.contacts.remove(at: index)
}
Copy link
Author

Choose a reason for hiding this comment

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

전에는 view 차원의 index를 전달받아 목록을 변경했었는데요. view controller에서 Contact의 id를 전달해, 이후 계층부터는 id를 통해 객체에 식별할 수 있도록 구현을 수정했습니다.


func updateContact(at index: Index, with newContact: Contact) throws {
guard validateIndex(index) else { throw ContactListError.invalidIndex }
func updateContact(with newContact: Contact) throws {
let index = try getIndexofContact(id: newContact.id)
self.contacts[index] = newContact
}
}

extension ContactList {
private func validateIndex(_ index: Index) -> Bool {
return self.contacts.indices.contains(index)
private func getIndexofContact(id: Int) throws -> Index {
guard let index = self.contacts.firstIndex(where: { contact in contact.id == id }) else {
throw ContactListError.invalidID
}
return index
Comment on lines +48 to +52
Copy link
Author

Choose a reason for hiding this comment

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

뷰 차원의 index을 validate 하는 구현을 제거하고 id를 통해 source Contact 배열 차원의 index를 구하는 구현을 추가했습니다.

}
}
Loading