프로젝트 관리 앱 [STEP 2-1] Ari #83
Conversation
- Project의 상태를 나타내는 ProjectState 열거형 타입 추가
- 기본값이 할당되어있던 id, date의 값 할당을 제거 - extension으로 init을 추가
- ViewModel 폴더 추가
- 기한이 지났는지 판단하는 프로퍼티 추가 - Date 타입을 String으로 반환해주는 프로퍼티 추가
- 기존 ViewModel의 구조를 List 형태로 변경
- info.plist 경로를 다시 ProjectManager 하위로 이동
- 각 기능들에 대해서 성공, 실패에 따른 테스트 코드 추가
- .swiftlint.yml 파일 수정
- fetch 메소드 추가
- 저장소에 관한 interface가 포함된 UseCase 프로토콜 추가
- MemoryStorage 추가에 따른 MemoryRepository 수정 - Storageable 프로토콜 추가
- 기존 MemoryRepositoryTests 코드 제거
- 데이터 바인딩을 하는 Observable 타입 추가 - 배열을 안전하게 조회하기 위해 Collection extension 추가
- 반환값이 없는데, 와일드카드 패턴 써준 부분 제거 - update 메소드 파라미터 알맞게 수정
- Observable을 제거하고 PublishSubject로 타입 변경 - 타입 수정에 따른 메소드 리팩토링
There was a problem hiding this comment.
아리, 반응형 프로그래밍 세계에 오신 것을 환영합니다. 생소한 개념을 처음 학습해보시고 적용해보시느라 고생 많으셨어요.
질의응답
궁금했던 것 / 조언을 얻고 싶은 점
클린 아키텍처에 맞춰서 제가 만든 타입들이 잘 설계된 것이 맞는지.. 라이언의 의견은 어떠신지 여쭤보고 싶습니다.
이미 회의실에서 이야기 나눈 바와 같이, Clean architecture에서 사용되는 용어들을 활용하고 있지만, 약간 어색한 점들이 있기는 합니다. 예를 들어, Repository와 Storage가 동일한 프로토콜 요구사항과 구현 내용을 가지고 있는 점이 있네요. 자세한 내용은 이야기 나누었으니 리팩토링을 거친 후에 다시 이야기해보면 좋을 것 같습니다.
현재의 ViewModel 구조가 적절한 구조인지 라이언의 조언이 필요합니다...🥲
ViewModel입장에서 input과 output을 다시 생각해보고, 어떤 것을 View로부터 받아야 useCase로 잘 전달해줄 수 있을지, useCase에서 받은 내용을 어떤 방식으로 View에 전달할 수 있도록 발행(publish)할 수 있는지 고민해보신다면 현재 구조에서 어떻게 변경해야할지 답이 나올거에요. 개별 코멘트도 한 번 참고해보세요~
ViewModel도 테스트해보려고 해보았는데... 아직은 기능이 단순해서인지.. 아니면 제가 설계를 이상하게[?] 한건 아닌지.. 제대로 한게 맞는지 감이 안잡히네요. 한번 살펴봐주시면 감사하겠습니다!
ViewModel을 리팩토링한 후 이야기 나누면 더 좋을 것 같아요. 미리 간략하게만 이야기하면 Mock 형식의 UseCase를 구현하여 ViewModel에 주입한 후 onNext 이벤트를 통해 각 subject에 이벤트를 전달하면 원하는 동작이 바인딩되어 잘 수행되는지 확인해보면 될 것 같네요.
회의실에서 이미 아래 내용들에 대해 이야기 나누어 보았으니, 아래 질문들은 회고에 활용해보시면 좋을 것 같습니다.
좋았던 점
- 구조가 Clean architecture에 기반해 나뉘어 있어 탐색하기 편했습니다.
- Clean architecture를 학습해보시고 적용을 도전해본 점
- 단위 테스트를 고려하여 테스트 가능한 구조로 설계하고 구현하신 점
질문
- Repository가 Data layer에서 떨어진 이유?
- memory의 의미?
- Memory storage, memory repository의 용도?
- 희망 RxSwift 활용 정도
- CUD 시 completion을 사용하는 이유
- ViewModel의 input/output 기준?
- XCTAssertTrue를 선호하는 이유
보완할 수 있는 점
- 한 요소를 표현하는 엔티티는 하나가 아니어도 됨. 엔티티 분리 Network (DTO) / CoreData / Domain
- 테스트 메서드명 보완
- MemoryStorage 보다 더 명확한 이름 (역할을 기반으로 - 이 앱의 메모리스토리지는 향후에도 이 친구 하나일까?)
- Filter.first < first(where:) / id값이 있으므로 indices를 살필 필요 없지 않은가?
- RxSwift 사용 시 Completion 대신 대안 활용 가능 (여기에선 Single이 적합해보임)
- Repositoryable, Storageable, UseCase보다 적합한 이름 찾기 (명사형도 하나의 옵션)
- Input도 Output처럼 메서드보다는 스트림 형식으로 전달되는 것이 적절하지 않을까요?
- 접근제한 설정
그럼 계속해서 화이팅입니다! 💪 궁금한 점 있으시면 언제든지 말씀해주세요~ 😉
| @@ -0,0 +1,33 @@ | |||
| import Foundation | |||
|
|
|||
| final class MemoryRepository { | |||
There was a problem hiding this comment.
타입의 특징을 이름으로 하는 것도 좋지만, 역할을 기반으로 이름을 정해주는건 어떨까요? MemoryRepository라면, 앞으로 유사한 역할을 수행해야하는 타입의 이름을 짓기 어려울 수 있을 것 같아요.
There was a problem hiding this comment.
VolatileRepository 라는 이름은 어떨까요??
(휘발성이라는 뜻으로...)
There was a problem hiding this comment.
타입이 어떤 특징을 가지고 있는지 설명하는 것도 나쁘지 않지만 어떤 역할을 수행하는지 (어떤 엔티티를 관리하는가? 등)에 대한 설명을 추가해서 이름을 지어보는건 어떨까요?
| extension MemoryStorage: Storageable { | ||
| func create(_ item: Project, completion: ((Project?) -> Void)?) { | ||
| projects.append(item) | ||
| completion?(item) |
There was a problem hiding this comment.
추가, 수정, 삭제 작업에 completion을 이용하여 입력받은 인스턴스를 그대로 전달해주는 이유는 무엇인가요? 의도가 궁금합니다~
There was a problem hiding this comment.
단순히 사용하기 편리해서 completion을 정의해주었는데, 저번에 회의실에서 추천해주셨던, completion을 대체할 수 있는 Single 타입으로 모두 리팩토링 진행하였습니다!
There was a problem hiding this comment.
이 메서드가 반환한 Single은 어떤 타입에서 누가 구독해서 이용하나요? 계획이 궁금합니다~
| projects.indices | ||
| .filter { projects[$0].id == item.id }.first |
There was a problem hiding this comment.
- 프로젝트 인스턴스들은 개별 인스턴스를 식별하는 UUID타입의 id를 가지고 있으니 인덱스를 기준으로 탐색하지 않아도 될 것 같네요.
filter.first보다first(where:)가 성능면에서 더 좋을 수 있으니 고려해보는 것도 좋겠어요!
There was a problem hiding this comment.
오.. 그렇네요 한번 이부분도 추가해보도록 하겠습니다!
의견 감사합니다! 😁
There was a problem hiding this comment.
그리고 filter.first의 경우 인덱스 값이 필요한 상황이여서. firstIndex로 대체하였습니다. 감사합니다 😁
| @@ -0,0 +1,8 @@ | |||
| import Foundation | |||
|
|
|||
| protocol Storageable { | |||
There was a problem hiding this comment.
프로토콜이라는 점을 강조하기 위해 -able 형식으로 마무리를 지으신 것으로 추측되네요. 무언가를 할 수 있다는 -able, -ible 형식 이외에도 -ing, 어떠한 역할을 수행한다는 의미에서 명사형의 이름도 많이 사용하니(Collection, Sequence, Publisher, ...) 자연스러운 이름을 지어보는 것도 좋겠어요.
There was a problem hiding this comment.
하하.. 네이밍은 언제나 어렵네요. 다시한번 고민해보도록 할게요 😅
| init(title: String, description: String, date: Date) { | ||
| self.init( | ||
| id: UUID(), | ||
| title: title, | ||
| description: description, | ||
| date: Date(), | ||
| status: .todo | ||
| ) | ||
| } |
There was a problem hiding this comment.
아래 방식으로 작성하는 것과 차이가 있나요? 저에게도 알려주세요~
init(
id: UUID = UUID(),
title: String,
description: String,
date: Date = Date(),
status: ProjectState = .todo
) {
self.init(
id: id,
title: title,
description: description,
date: date,
status: projectState
)
}There was a problem hiding this comment.
라이언이 적어주신 방식과 별 차이는 없지만, struct의 경우 따로 init을 적지 않아도 기본 이니셜라이저가 생성되는 것으로 알고 있습니다.
그래서 기본 이니셜라이저를 사용함과 동시에 extension으로 init을 추가하여 기본 이니셜라이저 말고도 추가된 이니셜라이저가 있다는 것을 의미하기 위해서 저런식으로 코드작성을 했던 것 같습니다.
There was a problem hiding this comment.
현재 작성하신 내용을 보면 date는 주입 받은 인스턴스를 사용하지 않고 항상 Date()를 사용하고 있네요!
| // MARK: - input | ||
| func add(_ project: Project) { | ||
| useCase.create(project) { item in | ||
| guard item != nil else { | ||
| self.errorMessage.onNext("알 수 없는 에러가 발생했습니다.") | ||
| return | ||
| } | ||
| self.projects = self.useCase.fetch() | ||
| self.inserted.onNext(true) | ||
| } | ||
| } | ||
|
|
||
| func update(_ project: Project, indexPath: IndexPath) { | ||
| useCase.update(project) { item in | ||
| guard item != nil else { | ||
| self.errorMessage.onNext("존재하지 않는 Project 입니다.") | ||
| return | ||
| } | ||
| self.projects = self.useCase.fetch() | ||
| self.updated.onNext(indexPath) | ||
| } | ||
| } | ||
|
|
||
| func delete(_ indexPath: IndexPath, completion: ((Project?) -> Void)?) { | ||
| useCase.delete(projects[safe: indexPath.row]) { item in | ||
| guard let item = item else { | ||
| self.errorMessage.onNext("삭제를 실패했습니다.") | ||
| completion?(nil) | ||
| return | ||
| } | ||
| self.projects = self.useCase.fetch() | ||
| self.deleted.onNext(indexPath) | ||
| completion?(item) | ||
| } |
There was a problem hiding this comment.
Subject 요소들을 잘 정의하고, 생성자에서 바인딩해준다면 이 메서드들은 제거할 수 있을 것 같아요.
|
|
||
| class MemoryUseCaseTests: XCTestCase { | ||
|
|
||
| var sut = MemoryUseCase() |
There was a problem hiding this comment.
- 프로토콜을 통해 추상화를 해주었으니 최대한 활용해보는 것도 나쁘지 않겠어요.
var sut: UseCase?- 각 테스트 케이스(메서드) 실행 전후로 sut 인스턴스를 동일하게 초기 상태로 만들어주기 위해
setUpWithError(),tearDownWithError()를 활용해볼 수 있을 것 같아요.
There was a problem hiding this comment.
초기 상태로 만들어주지 않아도 테스트에는 지장이 없어서 일부로 빼두었었는데요.
이 부분도 저번에 온라인 리뷰 듣고 개선해보았습니다. 의견 감사합니다~ 😁
| XCTAssertTrue(sut.fetch().count == 1) | ||
| } | ||
|
|
||
| func test_create_3() { |
There was a problem hiding this comment.
given, when, then 기반으로 Behavior Driven Development (BDD) 방식을 취했으니 해당 내용으로 메서드 이름을 지어준다면 그 자체가 해당 타입의 기능 명세가 될 수 있겠네요. 어떤 테스트를 통해 어떤 기능을 검증하는 것인지 알아볼 수 있도록 더 명확한 메서드 이름을 지어주는 것이 좋겠어요.
There was a problem hiding this comment.
제목은 간단할 수록 좋을 거 같다는 제 생각은 큰 오산이였네요 ㅋㅋㅋ
제가 생각해도 저 제목은... 애매한 것 같아요. 라이언 의견 듣고 테스트케이스 제목도 개선해보았습니다.
given, when, then 기반으로 이름을 지으니 수월했어요!
꿀팁 감사합니다 😊
| let project = Project(title: "제목", description: "상세 내용", date: Date()) | ||
| sut.create(project, completion: nil) | ||
| let newProject = Project(id: project.id, title: "수정", description: "수정 내용", date: Date(), status: .doing) |
There was a problem hiding this comment.
여러 프로젝트 인스턴스가 존재하는 상태에서 특정 id를 가지는 인스턴스를 수정하였을 때 해당 id를 가진 인스턴스가 수정됐다는 것을 검증해보는 것도 의미 있을 것 같아요.
There was a problem hiding this comment.
네! 이부분도 추가하여 개선해보았습니다!
추가로 Rx타입은 테스트를 어떻게 해야하나.. 찾아보다가 RxBlocking이란걸 사용하게 되었는데... 이 부분이 약간 어색해보이는 것 같아서.. 조언좀 부탁드리겠습니다!
There was a problem hiding this comment.
이벤트를 발행하는 인스턴스라면 구독을 통해 수신한 값을 검증하는 형태로 수행하면 됩니다~!
| XCTAssertTrue(item?.title == "제목") | ||
| } | ||
| XCTAssertTrue(sut.fetch().count == 1) |
There was a problem hiding this comment.
XCTAssertEqual(_:_:)보다 XCTAssertTrue(_:)를 선호하시는 것 같네요. 이유가 있나요~? 순전히 취향이라 무엇이 더 좋다, 맞다 같은 개념은 아니라는 것을 먼저 말씀드려요!
There was a problem hiding this comment.
이유가 특별하게 있는 것은 아니지만, 취향인 것 같습니다.
Equal보다 True가 읽기 더 편해보여서.. 였습니다 😄
- MemoryStorage 변경에 따른 Repository, UseCase 모두 반환타입 수정 - 테스트 코드 전반적인 리팩토링
#5 - .current에서 .autoupdatingCurrent로 수정
|
안녕하세요. 라이언🦁 수정사항
질문Input도 Output처럼 스트림 형식으로 개선해보기 실패
Single로 리팩토링 하면서 발생한 테스트 코드 경고문제
DateFormatter에 대한 성능이슈
이외 네이밍 개선 부분
|
- SwiftLint 함수 길이 제한 룰 제거
|
아리, 고생 많으셨습니다! 지금은 UI 요소가 없는 상태에서 Rx를 다루다보니 생소하게 느껴질 수 있지만, 프로젝트를 진행해 나가시면서 더 친숙해지실 거에요. 질문해주신 내용은 개별 코멘트와 아래 내용을 참고해보세요.
다음 스텝에서 만나요~~ 화이팅 ! 💪 |
안녕하세요. 라이언(@ryan-son) 🦁 아리(@leeari95)입니다~
ViewModel 까지 구현하고 PR 보내드립니다.
알아보기 쉽도록 UML도 끄적여보았어요...ㅎㅎ
처음에 Rx를 활용해서 ViewModel을 구현해보려고 해보았는데.. 적절한건지 봐주시면 감사하겠습니다.
알려주신 RxDataSource도 봤는데, 아직 Rx가 익숙치가 않아서.. 어렵네요. 😅
ViewModel을 각 TableView마다 매핑시켜줄 생각으로 설계를 해보았습니다.
처음이라 부족하겠지만... 잘부탁드리겠습니다🙇🏻♀️
UML
고민했던 점
ProjectMemoryStorageMemoryRepositoryMemoryUseCaseProjectListViewModel궁금했던 것 / 조언을 얻고 싶은 점
제대로 한게 맞는지 감이 안잡히네요. 한번 살펴봐주시면 감사하겠습니다!