[산군] 2, 3단계 자동 DI 미션 제출합니다 by s9hn · Pull Request #38 · woowacourse/android-di

s9hn · 2023-09-13T05:17:57Z

안녕하세요 해시
열심히 해봤는데 3단계에서 벽을느껴서 시간안에 구현하지 못했습니다..
(근로회의도 껴있어서 미리 제출했습니다)
최소한으로 구현은 해봤으나, 다음과 같은 문제가 있어서 롤백했습니다.

의존성 순환 문제를 해결하지 못했다.
뷰모델이 데이터소스의 출처를 알고있다.

다음과 같은 문제를 해결하기 위해서 현재 구조를 모두 바꿀 계획입니다.
이번 주 Step4 진행하면서 빠르게 리뷰 요청해보겠습니다 감사합니다
아 혹시 해시는 Qualifier 구분(String, 타입, 어노테이션 클래스 이름 등)을 어떻게 해주었나요

2단계 요구사항

필수

ViewModel 내 필드 주입을 구현한다..
의존성 주입이 필요한 필드와 그렇지 않은 필드를 구분할 수 없다.
- Annotation을 붙여서 필요한 요소에만 의존성을 주입한다.
- 내가 만든 의존성 라이브러리가 제대로 작동하는지 테스트 코드를 작성한다.
CartRepository가 다음과 같이 DAO 객체를 참조하도록 변경한다.
CartProductViewHolder의 bind 함수에 다음 구문을 추가하여 뷰에서도 날짜 정보를 확인할 수 있도록 한다.

선택

현재는 장바구니 아이템 삭제 버튼을 누르면 RecyclerView의 position에 해당하는 상품이 지워진다.
- 상품의 position과 CartRepository::deleteCartProduct의 id가 동일한 값임을 보장할 수 없다는 문제를 해결한다.
뷰에서 CartProductEntity를 직접 참조하지 않는다.

3단계 요구사항

필수

하나의 인터페이스의 여러 구현체가 DI 컨테이너에 등록된 경우, 어떤 의존성을 가져와야 할지 알 수 없다.
- 상황에 따라 개발자가 Room DB 의존성을 주입받을지, In-Memory 의존성을 주입받을지 선택할 수 있다.
내가 만든 DI 라이브러리를 모듈로 분리한다.

선택

DSL을 활용한다.
내가 만든 DI 라이브러리를 배포하고 적용한다.

EmilyCh0

아직 요구사항 만족하지 않은 부분이 많아서 빠르게 리뷰 남깁니다! 다음 리뷰 요청 때는 필수 요구사항 꼭 완성해주세요! 💪

내가 만든 의존성 라이브러리가 제대로 작동하는지 테스트 코드를 작성한다
Recursive DI 구현
하나의 인터페이스의 여러 구현체가 DI 컨테이너에 등록된 경우, 어떤 의존성을 가져와야 할지 알 수 없다.
- 상황에 따라 개발자가 Room DB 의존성을 주입받을지, In-Memory 의존성을 주입받을지 선택할 수 있다.
내가 만든 DI 라이브러리를 모듈로 분리한다.

EmilyCh0 · 2023-09-13T07:42:26Z

+        val db = DatabaseModule.providesShoppingDatabase(this)

        StartInjection {
-            single<CartRepository>(DefaultCartRepository())
-            single<ProductRepository>(DefaultProductRepository())
+            single<CartRepository>(RepositoryModule.provideCartRepository(db.cartProductDao()))
+            single<ProductRepository>(RepositoryModule.provideProductRepository())


provideXxx() 함수를 모듈에 이미 다 만들어놨으니 Application에서 하나 하나 호출해서 DependencyContainer에 add하기 위한 코드를 작성하지 않아도 될 것 같은데 산군은 어떻게 생각하시나요? provideSth() 함수가 있지만 Application에 추가하는 것을 빼먹어 의존성 주입에 필요한 인스턴스가 없는 경우가 생길 수 있지 않을까요?

모듈에 대한 이해가 부족했던 것 같아요!
저는 지금 제 방식이 코인에 가깝다고 생각하고, 모듈내에서 프로바이드 함수호출로 인스턴스를 주입해주는 방식이 힐트에 가깝다고 생각해요
다음 리팩터링엔 현재 코인방식을 모두 힐트방식으로 바꿔볼 예정입니다!

EmilyCh0 · 2023-09-13T08:03:26Z

+
+    inline fun <reified T : Any> inject(): T {
+        val clazz = T::class
+        val constructor = clazz.primaryConstructor ?: throw IllegalArgumentException()


예외 던질 때 메시지를 함께 남기면 어디에서 오류가 발생했는지 확인하기 쉬울 것 같아요! (아래 코드도)

EmilyCh0 · 2023-09-13T08:05:40Z

+                    val argument =
+                        DependencyContainer.repositories[type] ?: throw IllegalArgumentException()


DependencyContainer의 repositories에서 직접 꺼내오기 보다는 DependencyContainer에게 해당 타입의 인스턴스를 달라고 하는 게 어떨까요?

함수를 통해 받아오는 일종의 게터구현방식이 될 것 같은데, 해당 리뷰에 대해 좀 더 첨언해주실 수 있으실까요?!

우선 외부에서 repositories 전체를 가져다 사용해야하는 경우가 있나요? 없다면 repositories를 통채로 외부에 노출 시킬 필요가 없다고 생각해요. 그리고 인스턴스를 찾는 로직은 DependencyContainer의 관심사라고 생각할 수도 있을 것 같아요. DependencyContainer 내부 구현이 바뀌어서 Map을 사용하지 않게 된다면 repositories에서 인스턴스를 꺼내 사용하는 곳이 모두 변경되어야 할까요?
( + repositories에서 인스턴스를 꺼낼 때마다 toMap()으로 새로 맵을 만드는 비용도 있을 것 같아요! repositories 맵 전체가 필요한 게 아니라면 불필요한 비용이 아닐까싶습니다)

EmilyCh0 · 2023-09-13T08:22:11Z

+        return constructor.call(*arguments.toTypedArray()).apply {
+            injectField<T>(this)
+        }


클래스 생성자에 의존성 주입이 필요하지 않은 파라미터가 있는 경우는 없을까요? (디폴트 값이 있다거나..)

EmilyCh0 · 2023-09-13T08:24:37Z

+    @WooWaField
+    private lateinit var cartRepository2: CartRepository


필드 주입 테스트하던 코드인거 같네요 😅

EmilyCh0 · 2023-09-13T08:27:07Z

의존성 주입에 사용되는 어노테이션이 util 패키지에 있는게 적절할까요? DI 라이브러리를 분리한다고 생각했을 때 라이브러리에 정의될 어노테이션이라고 생각하는데 산군은 어떻게 생각하시나요?

EmilyCh0 · 2023-09-13T08:28:05Z

+@Target(AnnotationTarget.CONSTRUCTOR, AnnotationTarget.PROPERTY)
+annotation class WooWaInject


WooWaInject를 프로퍼티에 붙이는 경우는 어떤 경우일까요? WooWaField를 붙이는 것과 다른걸까요?

EmilyCh0 · 2023-09-13T08:28:58Z

+@Target(AnnotationTarget.CLASS)
+annotation class WooWaLazyInject


지연초기화는 어떤 경우에 사용되나요? (궁금)

EmilyCh0 · 2023-09-13T08:30:51Z

의존성 주입 테스트가 아니네요...😢

EmilyCh0 · 2023-09-13T09:04:17Z

구현 방식이 달라서 도움이 될지 모르겠지만 혹시 힌트가 될까해서 제가 구현한 방식 아이디어만 살짝 남깁니다!

Qualifier

아 혹시 해시는 Qualifier 구분(String, 타입, 어노테이션 클래스 이름 등)을 어떻게 해주었나요

저는 타입과 Qualifier 정보를 함께 String에 담아 인스턴스의 식별 Key로 사용했어요.
그리고 라이브러리를 사용하는 개발자가 Qualifier를 새로 만들고 싶다면 annotation class를 새로 정의하고, 아래 InMemory처럼 @Qualifier 어노테이션을 붙여 사용할 수 있도록 했어요.

annotation class Qualifier

@Qualifier
annotation class InMemory

Recursive DI

저는 모듈에 정의된 함수를 돌면서 인스턴스를 먼저 컨테이너에 담아두고 사용했어요. (4단계 진행하면서 변경될 예정..) 컨테이너에 담는 과정에 의존성 주입이 필요한 인스턴스를 재귀로 구합니다. (의존성 주입이 필요없는 인스턴스까지 createInstance()를 타고 들어가는 방식..)

module::class.declaredMemberFunctions.forEach { kFunc ->
    appContainer[kFunc의 Key] = createInstance(...)
}

private fun createInstance(...): Any? {
		
    val injectParams = // inject 필요한 파라미터 찾기

    if (injectParams.isNotEmpty()) { // inject 필요한 파라미터가 있다면
        injectParams.forEach { param ->
			// appContainer에 해당 인스턴스가 있는지 확인하고
			// appContainer에 없다면 `appContainer[key] = createInstance(...)` 로 재귀
        }
        // 이 시점에는 injectParams에 해당하는 인스턴스가 appContainer에 모두 저장된 상태!
        val args = // injectParams에 해당하는 인스턴스들을 appContainer에서 꺼내온다

	...

        return provideFunc.callBy(args) // 인스턴스 생성해서 리턴
    }
		
    return // inject 필요한 파라미터 없으면 그냥 인스턴스 생성해서 리턴
}

s9hn · 2023-09-14T18:48:37Z

안녕하세요 해시! 리뷰남겨줘서 감사합니당
저는 step3가 힘들었던게 데이터소스를 추상화하는줄 알고 좀 어려워했었는데,
그게 아니라 레포지토리만 추상화였군요!;; 일단 구현은 했습니다!
물론 데이터소스 또한 나중에 추상화되었을 때 정상적으로 주입받기 위해 코드 리펙터링은 불가피해보입니다!

정작 step3는 안하고 이것저것 시도를 해보려는 탓에 레거시 코드가 많이 묻어있었네요ㅎ;;
리뷰대로 다 없애주었습니다 b
그리고 위의 의존성 순환문제와 생명주기 관리 등으로 인해, 기존 DI 방식에서 새로운 방식으로 TDD 리팩터링 해보고 있습니다.
이는 시간이 조금 걸릴 것 같아 최대한 해보고 주말에 꼭 PR올려보겠습니다.

EmilyCh0 · 2023-09-15T02:54:05Z

WooWaQualifier랑 InMemoryCartRepository가 없어서 빌드 할 수가 없어요! 푸시 한번 다시 해주세용

s9hn · 2023-09-15T05:41:08Z

!
작업중이던게 있어서 안올렸는데 깜빡했네유 쏴리 ㅋㅋ

EmilyCh0

미션 고생하셨습니다! 아직 남아있는 요구사항은 4단계에서 전체적으로 구조 변경하면서 반영한다고 하셔서 머지하겠습니다~

EmilyCh0 · 2023-09-15T06:19:18Z

+        const val DATABASE = "DATABASE"
+        const val IN_MEMORY = "IN_MEMORY"


현재 DependencyContainer에서 이 string 값을 Map의 Key로 사용하고 있어서 CartRepository에만 사용 할 수 있어요. 만약 ProductRepository도 Database와 InMemory 구현체가 존재한다면 어떻게 될까요?

EmilyCh0 · 2023-09-15T06:35:52Z

        this.block()
    }

+    inline fun <reified T : Any> single(qualifier: String, instance: Any) {


여긴 T가 사용되지 않네요!

s9hn added 5 commits September 12, 2023 01:07

feat: 어노테이션 추가

06bd99f

feat: 필드주입 구현 성공 및 어노테이션 테스트

d5fb7f5

feat: 룸적용!

f068efa

refactor: 패키지 이동

2f9bed3

feat: 모듈분리

e2a64a4

EmilyCh0 requested changes Sep 13, 2023

View reviewed changes

s9hn added 4 commits September 14, 2023 23:51

chore: 디렉터리명 수정

9c21506

feat: step3 구현

6038deb

refactor: 패키지 이동 및 필요없는 어노테이션 삭제

3cc40cf

refactor: 인젝터 에러문구 추가 및 코드 리팩터링

d2d7407

refactor: createAt 기본값 추가

57f4536

feat: InMemoryCartRepository 추가

91c9b48

EmilyCh0 approved these changes Sep 15, 2023

View reviewed changes

EmilyCh0 merged commit 9a5b093 into woowacourse:s9hn Sep 15, 2023

EmilyCh0 mentioned this pull request Sep 20, 2023

[산군] 4단계 자동 DI 미션 제출합니다 #64

Merged

		val argument =
		DependencyContainer.repositories[type] ?: throw IllegalArgumentException()

		@WooWaField
		private lateinit var cartRepository2: CartRepository

		@Target(AnnotationTarget.CONSTRUCTOR, AnnotationTarget.PROPERTY)
		annotation class WooWaInject

		@Target(AnnotationTarget.CLASS)
		annotation class WooWaLazyInject

		const val DATABASE = "DATABASE"
		const val IN_MEMORY = "IN_MEMORY"

Conversation

s9hn commented Sep 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

2단계 요구사항

필수

선택

3단계 요구사항

필수

선택

Uh oh!

EmilyCh0 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EmilyCh0 commented Sep 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

s9hn commented Sep 14, 2023

Uh oh!

EmilyCh0 commented Sep 15, 2023

Uh oh!

s9hn commented Sep 15, 2023

Uh oh!

EmilyCh0 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

s9hn commented Sep 13, 2023 •

edited

Loading

EmilyCh0 commented Sep 13, 2023 •

edited

Loading