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

홈 화면에서 카테고리 별로 책 필터링하여 보여주기 #73

Merged
merged 42 commits into from
Nov 25, 2024

Conversation

Kyxxn
Copy link
Member

@Kyxxn Kyxxn commented Nov 24, 2024

#️⃣ 연관된 이슈


⏰ 작업 시간

예상 시간 실제 걸린 시간
3.3 (5) 8 (동시성 문제 해결하느라.. ㅂㄷㅂㄷ)

📝 작업 내용

  1. 주요 기능 추가
  • MemorialHouse 모델 구현 및 UseCase 추가.
  • DefaultFetchUserHouseUseCase에 닉네임 사이 공백 추가 로직 구현.
  • ViewDidLoad 시점에 UserHouse를 Fetch 하여 UI 업데이트.
  • HomeViewModelFactory 추가로 Register -> Home 연결.
  • 카테고리별 홈 화면 책 필터링 로직 구현.
  • 카테고리 시트지 선택 시:
    • 홈 화면의 현재 카테고리 UI 변경.
    • 현재 선택 중인 카테고리에 체크 표시 추가.
  1. 리팩토링
  • DIContainer.shared를 MainActor로 변환.
  • SceneDelegate에서 houseName 전달 로직 제거.
  • MHNavigationBar의 Title 설정 방식을 생성 시점에서 configure 메소드로 변경.

📸 스크린샷

스크린샷 2024-11-25 오전 2 33 27 스크린샷 2024-11-25 오전 12 50 59

홈화면 필터링 구현

⚽️ 트러블 슈팅

MHDomain이라는 모듈 안에 MHDomain 동적 라이브러리MHDomainTests 정적 라이브러리가 있고,

MHPresentation 모듈 안에 MHPresentation 동적 라이브러리MHPresentationTests 정적 라이브러리가 있다.

테스트를 진행할 때 import MHDomain 을 해주어야 하는데,

이 때 import 앞에 @testable 키워드를 사용할 수 있다.

무슨 역할을 하는 지 알아보자


문제 해결

@testable 키워드

테스트 대상 모듈의 접근 수준에 영향을 주는지 여부이다.

  1. 테스트 접근성 증가:
  • Swift의 기본 접근 수준은 internal인데, 즉 동일 모듈 내에서만 접근할 수 있는 게 기본이다.
  • @testable import를 사용하면 테스트 모듈에서 internal 멤버를 접근할 수 있게 되어, 단위 테스트 작성이 훨씬 용이해진다.
  1. 테스트 커버리지 향상:
  • 일반 import로는 테스트할 수 없는 모듈 내부의 구현 세부 사항(internal 멤버)도 테스트할 수 있다.
  • 단위 테스트에서 비즈니스 로직이나 헬퍼 메서드 등을 보다 직접적으로 검증할 수 있다.
스크린샷 2024-11-25 오전 8 32 58

실제로 비즈니스 로직을 테스트 해야하는데 어떻게 해야할 지 몰라서 여쭤봤었고,

그래서 private 혹은 internal 메소드를 다른 타겟에서 사용한다는 관점을 빌려 public으로 바꾸려 했었다가 팀원의 조언으로 멈췄다.

빛정현 ;;;

실제 적용사례

MHPresentation 모듈 안에 MHPresentation 동적 라이브러리MHPresentationTests 정적 라이브러리가 있다.

MHDomain 테스트할 때는 외부 모듈의 접근이 잦으니 public으로 되어있는 경우가 많은데,

MHPresentation는 대부분 internal로 구현이 되어 있다.

public final class HomeViewModel: ViewModelType {
    func transform(input: AnyPublisher<Input, Never>) -> AnyPublisher<Output, Never> {
        input.sink { [weak self] event in
            switch event {
            case .viewDidLoad:
                self?.fetchMemorialHouse()
            case .selectedCategory(let index):
                self?.filterBooks(with: index)
            }
        }.store(in: &cancellables)
        
        return output.eraseToAnyPublisher()
    }

위 코드의 경우 그러면 저 transform을 public으로 해줄 거냐 ?

protocol ViewModelType {
    associatedtype Input
    associatedtype Output
    
    @MainActor
    func transform(input: AnyPublisher<Input, Never>) -> AnyPublisher<Output, Never>
}

그러면 위 프로토콜 자체도 public으로 바꿔줘야 한다.

이게맞나..?

라는 생각이 들 때 위 개념이 떠올라서 @testable을 붙이니까 MHPresentation 내부에 있는 internal 메소드도 사용이 가능했다 ^◡^

Kyxxn added 22 commits November 22, 2024 18:39
DIContainer가 nil과 throw를 반환하기 때문에 initialViewController를 기존에 초기화 해두고, 이후 UserDefaults에 값이 있다면 do-catch를 진행했습니다
@Kyxxn Kyxxn linked an issue Nov 24, 2024 that may be closed by this pull request
@Kyxxn Kyxxn changed the title Feature/filtering book 홈 화면에서 카테고리 별로 책 필터링하여 보여주기 Nov 24, 2024
@Kyxxn Kyxxn requested a review from k2645 November 24, 2024 14:17
@Kyxxn Kyxxn self-assigned this Nov 24, 2024
@Kyxxn Kyxxn added the ✨ Feature 기능 관련 작업 label Nov 24, 2024
@Kyxxn Kyxxn added this to the 0.3 milestone Nov 24, 2024
@Kyxxn Kyxxn marked this pull request as ready for review November 24, 2024 17:38
Copy link
Member Author

@Kyxxn Kyxxn left a comment

Choose a reason for hiding this comment

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

테스트 코드 환경 구축 & 테스트 2개 작성했더니 평소보다 파일 변화가 더 많아진 것 같습니다,,
다음부터 적게적게 올리겠습니다

Swift Testing을 진행해봤는데, 그 과정에서 알게된 내용 기술공유에 정리해두겠습니다
+아직 여러 태그를 비롯해서 Swift Testing을 잘 활용하고 있지 못하다는 생각이 드는데 더 공부해봐야겠네요.

Comment on lines +20 to +28
var initialViewController: UIViewController = RegisterViewController(viewModel: RegisterViewModel())
if UserDefaults.standard.object(forKey: Constant.houseNameUserDefaultKey) != nil {
do {
let viewModelFactory = try DIContainer.shared.resolve(HomeViewModelFactory.self)
let viewModel = viewModelFactory.make()
initialViewController = HomeViewController(viewModel: viewModel)
} catch {
MHLogger.error(error.localizedDescription)
}
Copy link
Member Author

Choose a reason for hiding this comment

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

기존에 UserDefaults에서 이름을 꺼내고 HomeViewModel에 넘겨주던 로직을 제거하고,
HomeViewModel이 CoreData에 저장되어 있는 MemorialHouse 엔티티 모델로부터 이름을 받아오는 것으로 변경했습니다

Comment on lines -1 to +2
public actor DIContainer {
public static let shared = DIContainer()
public final class DIContainer {
@MainActor public static let shared = DIContainer()
Copy link
Member Author

Choose a reason for hiding this comment

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

TODO: actor로 사용할 지, class + MainActor로 사용할 지 논의해야 함

@@ -0,0 +1,15 @@
public struct MemorialHouse: Sendable {
Copy link
Member Author

Choose a reason for hiding this comment

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

UserHouse에서 MemorialHouse로 네이밍 변경했습니다

Comment on lines +1 to 4
struct CategoryViewModel {
private(set) var categories: [String]
private(set) var currentCategoryIndex: Int
}
Copy link
Member Author

Choose a reason for hiding this comment

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

지금은 하는 일이 없어서 필요없어 보이지만,
나중에 편집 & 추가할 때 class로 변경하고 로직들이 추가될 예정입니다

Comment on lines +3 to +13
public struct HomeViewModelFactory {
let fetchMemorialHouseUseCase: FetchMemorialHouseUseCase

public init(fetchMemorialHouseUseCase: FetchMemorialHouseUseCase) {
self.fetchMemorialHouseUseCase = fetchMemorialHouseUseCase
}

public func make() -> HomeViewModel {
HomeViewModel(fetchMemorialHouseUseCase: fetchMemorialHouseUseCase)
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

DIContainer에 ViewModel이 아닌, ViewModelFactory 저장하기로 결정했다.

try await Task.sleep(nanoseconds: 500_000_000)

// Act 실행 단계: SUT 메소드를 호출하면서 의존성을 전달해서 결과를 저장하기
receivedOutputs.removeAll()
Copy link
Member Author

Choose a reason for hiding this comment

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

비워주지 않으면 위 테스트 메소드 func test카테고리_선택시_해당_카테고리에_맞는_책들로_필터링한다()에 의해 항상 실패하게 되는 것을 보고 추가해주었습니다

Copy link
Collaborator

@iceHood iceHood left a comment

Choose a reason for hiding this comment

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

주말을 불태운 당신께.... Cheers 🍷

Copy link
Collaborator

Choose a reason for hiding this comment

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

P2. 효준님! 테스트 타겟 버전도 저희 프로젝트 타겟 버전으로 맞추면 좋을 것같아요!
스크린샷 2024-11-25 오전 9 46 09

Copy link
Member Author

Choose a reason for hiding this comment

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

인정합니다 바로 반영할게요

Copy link
Collaborator

@k2645 k2645 left a comment

Choose a reason for hiding this comment

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

정말 수고 많으셨습니다 .ᐟ.ᐟ 코멘트 쫌쫌따리 남겨두었어용 ~

@@ -1,4 +1,4 @@
public enum BookColor {
public enum BookColor: Sendable {
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3: 엔티티에 Sendable을 붙여주는 이유가 무엇인가요??

Copy link
Member Author

Choose a reason for hiding this comment

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

스크린샷 2024-11-25 오후 2 42 17

붙이지 않으면 모델이 non-isolated 하다는 에러를 내뿜어서 일단 붙여둔 상태입니다..!

Comment on lines 4 to 16
public let identifier = UUID()
public let title: String
public let imageURL: String
public let bookColor: BookColor
public let color: BookColor
public let category: String
public let isLike: Bool
public let favorite: Bool

public init(
title: String,
imageURL: String,
bookColor: BookColor,
color: BookColor,
category: String,
isLike: Bool = false
favorite: Bool = false
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2: 이렇게 identifier를 프로퍼티에 넣어버리면 이니셜라이져 부분에서 identifier를 받을 수 없게 됩니다. 제.. 브랜치에서 이미 바꾸긴했는데,

Suggested change
public let identifier = UUID()
public let title: String
public let imageURL: String
public let bookColor: BookColor
public let color: BookColor
public let category: String
public let isLike: Bool
public let favorite: Bool
public init(
title: String,
imageURL: String,
bookColor: BookColor,
color: BookColor,
category: String,
isLike: Bool = false
favorite: Bool = false
public let identifier: UUID
public let title: String
public let imageURL: String?
public let color: BookColor
public let category: String?
public let favorite: Bool
public init(
identifier: UUID = .init(),
title: String,
imageURL: String?,
color: BookColor,
category: String?,
favorite: Bool = false

이렇게 하는게 좋을 것 같아욥

Copy link
Member Author

Choose a reason for hiding this comment

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

반영했습니다 !

@@ -0,0 +1,3 @@
public protocol MemorialHouseRepository: Sendable {
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3: 여기도 Sendable이 붙었군요.. 혹시 왜 붙었는지 알 수 있을까요??

Comment on lines 19 to 23
/// 집 이름이 3글자 이상일 경우, 각 글자 사이에 공백을 추가하여 변환합니다.
///
/// - Parameter name: 원본 이름 문자열.
/// - Returns: 글자 사이에 공백이 추가된 문자열(3글자 이상인 경우).
/// 2글자 이하라면 원본 문자열 그대로 반환합니다.
Copy link
Collaborator

Choose a reason for hiding this comment

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

옹 자세한 주석 너무 좋은 것 같습니다 .ᐟ.ᐟ 👍

@@ -0,0 +1,13 @@
@testable import MHDomain

public struct StubMemorialHouseRepository: MemorialHouseRepository {
Copy link
Collaborator

Choose a reason for hiding this comment

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

P1: TestDouble 객체는 test code 함수에서만 사용될 객체인데 public 접근제어자를 가진게 뭔가 어색해보입니다 ! 그냥 internal로 사용가능하지 않나요??

Copy link
Member Author

Choose a reason for hiding this comment

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

인정합니다 까먹고 제거 안 했네요..!
수정하겠습니다 !!

Comment on lines 24 to 28
private func transformHouseName(with name: String) -> String {
guard name.count > 2 else { return name }
return name.map { String($0) }.joined(separator: " ")
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

P1: 근데 로직이 반대입니다...ᐟ.ᐟ 2글자 이하일때 공백이 들어가고 3글자 이상부터는 붙여서 반환해야합니다 .ᐟ.ᐟ

Copy link
Member Author

Choose a reason for hiding this comment

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

?? 아하..! 멍청이슈..
수정하겠습니다

@@ -4,5 +4,6 @@ protocol ViewModelType {
associatedtype Input
associatedtype Output

@MainActor
Copy link
Collaborator

Choose a reason for hiding this comment

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

이곳에도...Swift6의 흔적이...ㄷㄷ;;

output.sink { [weak self] event in
guard let self else { return }
switch event {
case .fetchedMemorialHouse:
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2: viewmodel이 memorial house를 프로퍼티로 가지고 있기보단 해당 output이 memorialHouse에 대한 값을 넘겨주는 방식이 조금 더 자연스러울 것 같다는 생각입니다 ! viewmodel의 프로퍼티에 view가 직접 접근하는 방식은 최대한 지양하는게 좋을 것 같습니당

Copy link
Member Author

Choose a reason for hiding this comment

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

오.. 12있는 말씀이십니다
그러면 뷰컨이 프로퍼티로 들고있을 필요는 없고, 받아서 바로 UI 그리는데에만 쓰는 느낌으로 가는 건가요 ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

넵넵 맞습니다 .ᐟ.ᐟ

var sut: HomeViewModel!
var cancellables = Set<AnyCancellable>()

@MainActor
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3: test 코드에 mainactor가 붙은게 어색해 보이는데.. 이렇게 해주신 이유가 있으신가욥..?

Copy link
Member Author

Choose a reason for hiding this comment

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

HomeViewModel의 메소드들이 MainActor에서 동작하기 때문에
테스트 코드의 메소드들도 MainActor 키워드를 붙이지 않으면 에러가 발생했습니다 !
이번 PR은 사실 Actor 공부가 다 잘 안 돼서 비즈니스 로직에 중점을 두긴 했어요


// Act 실행 단계: SUT 메소드를 호출하면서 의존성을 전달해서 결과를 저장하기
input.send(.viewDidLoad)
try await Task.sleep(nanoseconds: 500_000_000)
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3: 중단 시간을 두는 이유가 무엇인가요?

Copy link
Member Author

Choose a reason for hiding this comment

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

input output 스트림에서 비동기로 동작하니까 지연시간을 줘서 이벤트를 확실히 받을 수 있게 했습니다 !

@Kyxxn Kyxxn requested a review from k2645 November 25, 2024 05:46
Copy link
Collaborator

@k2645 k2645 left a comment

Choose a reason for hiding this comment

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

정말 너무너무 수고 많으셨습니다 🥹

@Kyxxn Kyxxn merged commit 3cc254a into develop Nov 25, 2024
2 checks passed
@Kyxxn Kyxxn deleted the feature/filtering-book branch November 25, 2024 05:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ Feature 기능 관련 작업
Projects
None yet
Development

Successfully merging this pull request may close these issues.

카테고리별 책 구분
3 participants