[둘리] 4단계 자동 DI 미션 제출합니다 #57

hyemdooly · 2023-09-18T18:21:17Z

안녕하세요 베르! 이번에는 제가 베르보다 늦었네요!!ㅎㅎ
아크의 도움을 많이 받아서 구조가 비슷할거에요. 이번 미션은 혼자만의 생각으로는 어렵더라고요.🥲
필수 요구 사항을 더 신경써서 만족시키고 시간이 남으면 선택 요구 사항을 하고 싶어서, 선택 요구 사항은 아직 구현하지 않았어요.
처음 작성한 DI에 비해 많은 부분이 바뀌었는데 아래에 요약하겠습니다.

di

open class Module(private val parentModule: Module? = null) {
    private val instances = mutableMapOf<KClass<*>, Any>()

Module, Container, Injector로 나뉘던 것이 Module 하나로 합쳐졌습니다.
Module은 KClass를 키로 하는 인스턴스 맵을 가지고 있어요. 만약 어딘가에서 이 모듈에게 getInstance를 요청한다면 아래 순서로 확인합니다.

싱글톤에 있는가? (= 맵에 있는 인스턴스인가?)
모듈에 선언된 멤버 함수 중, 이 인스턴스를 리턴하는 함수가 있는가?
부모 모듈에 있는가?
셋 다 없으면 생성하기
그리고 injectField(instance: T) 함수를 열게 되었어요. 이번에 DateFormatter를 Activity에 주입해줘야해서, Activity 스스로 자기 자신 필드를 주입하려면 필요하다고 판단하여 열었습니다.

android-di

di를 implement하여 구현했습니다.
HyemdoolyActivity, HyemdoolyApplication, HyemdoolyViewModel이 존재합니다.
기본적으로 컴포넌트들은 자신에 맞는 모듈을 가지고 있어요. 이 모듈을 생성하는 ModuleFactory가 존재합니다.
Modules에는 각 단순한 모듈 생성 함수를 정의해줄 것이고, ModuleFactory는 더 넓게 modules를 필요로 하는 인스턴스들을 생성해요. (ex. HyemdoolyActivity에 들어가는 ViewModel)
여기서 고민인 부분이 HyemdoolyViewModel인데요. AAC ViewModel로 만들어서 Activity가 파괴되었다가 재생성되더라도 데이터 유지에 대응하기 위해 만든 부분입니다. (물론 아직 설정 변경은 적용하지 않았습니다.)
ViewModel이라고 하기에는 ActivityModule과 viewModelModule을 들고 있는게 전부라 네이밍이 애매한가?라는 생각이 듭니다. 하지만 생명주기 때문에 지금처럼 ViewModel()을 상속받고 싶다는 생각도 들어요. 이 부분은 더 장기적으로 고민해야할 것 같아요.
그리고 전 미션에서 베르가 자동으로 해줬으면 좋겠다고 하셨던 ViewModelFactory 부분! util에 ViewModelLazy를 이용한 생성으로 정의해두었습니다!

app

CartActivity의 DateFormatter는 개발자가 관리하지 않아도 되도록 @Inject를 달아 주입을 받게 시켰습니다.
Activity, Application의 부모 클래스를 모두 HyemdoolyXXX로 바꿨고, viewModels도 android-di의 viewModels를 사용했어요.
미션 요구 사항에 따라 ActivityModule, ApplicationModule, ViewModelModule을 정의했습니다. 이 모듈들은 실제 실행되는 컴포넌트 내에 있기 때문에, 그 컴포넌트가 파괴되면 함께 사라질거에요!

추가로 고민인 부분들

실제 정의한 Module 자식클래스들을 보면 interface가 아니라 구현체를 리턴해주고 있는데요!
Qualifier로 어떤 구현체를 넣을지 ViewModel에 정의되어있어서 Module도 구현체를 리턴하도록 했습니다.
Module의 멤버함수에도 Qualifier를 붙여준다면 리턴타입을 interface로 만들 수 있을 것 같은데 의미가 있을까요?🤔
DI 사용자가 계속 annotation을 붙이도록 하는게 불편하지 않을까 싶어서 더 변경하지 않았는데, 추상화된게 아니라 구현체를 리턴하는게 마음이 편하지는 않네요.

리뷰 잘 부탁드립니다!😁

SeongHoonC

둘리! 4단계 미션 쉽지 않았을텐데 고생많으셨습니다!
전체적으로 구조가 흥미로웠습니다! 저도 많이 배워가요!
리뷰 확인해주세요!

SeongHoonC · 2023-09-20T23:30:58Z

di/src/test/kotlin/io/hyemdooly/di/ModuleTest.kt

+ private val fakeChildModule = FakeChildModule(fakeParentModule)
+
+ @Test
+ fun `모듈에서 처음 사용하는 싱글톤 인스턴스는 생성 후 모듈이 가지고 있는 컬렉션에 저장한다`() {


컬렉션에 저장되는 것이 테스트 이름에 노출될 필요가 있을까요?

SeongHoonC · 2023-09-20T23:42:20Z

di/src/test/kotlin/io/hyemdooly/di/ModuleTest.kt

테스트를 봤을 때 무슨 기능을 제공하는지 한눈에 알아보기 힘들어요!
결국 어떤 것을 검증하기 위함인지 생각해보셨으면 좋겠습니다.

힌트 : 테스트는 실제로 내부가 어떻게 돌아가는지 관심 없어야 된다고 생각합니당

SeongHoonC · 2023-09-20T23:54:16Z

di/src/main/java/io/hyemdooly/di/Module.kt

+ private fun addInstance(instance: Any) {
+ instances[instance::class] = instance
+ }


사용하지 않는 함수인 것 같네요!

헉 그렇네여!

SeongHoonC · 2023-09-20T23:57:51Z

app/src/main/java/woowacourse/shopping/ui/ViewModelFactory.kt

viewModelFactory 가 없어지면 테스트도 리팩터링이 필요합니다!

SeongHoonC · 2023-09-21T00:00:27Z

di/src/main/java/io/hyemdooly/di/Module.kt

+ val key = instances.keys.firstOrNull { it.isSubclassOf(type) }
+ if (key != null && instances[key] != null) return instances[key]


위 부분이 없어도 모든 테스트가 통과하는데 어떤 것을 위한 코드인지 궁금합니다!

type에 해당하는 인스턴스가 없는 경우, 호환 가능한 인스턴스를 리턴하도록 작성한 코드인데요!
테스트코드 보완이 필요하겠네요!

SeongHoonC · 2023-09-21T00:03:45Z

app/build.gradle.kts

 // implementation("com.github.hyemdooly:android-di:v4.0.0")
 implementation(project(":di"))
+ implementation(project(":android-di"))


배포한 라이브러리를 포기하고 모듈로 변경한 이유가 있나요?

android-di도 배포해서 사용해야하는데 아직 새로 배포하지 않았습니다.ㅎㅎ

SeongHoonC · 2023-09-21T00:07:14Z

android-di/src/main/java/io/hyemdooly/androiddi/base/HyemdoolyViewModel.kt

+ val isInitailzeModule: Boolean
+ get() = ::activityModule.isInitialized && ::viewModelModule.isInitialized


사용하지 않는데 무엇을 위한 코드인가요?

구성변경 대응하려고 만들었다가 안지웠네요...

SeongHoonC · 2023-09-21T00:12:38Z

android-di/src/test/java/io/hyemdooly/androiddi/ComponentInjectTest.kt

Fake 객체들이 다 다른 파일에 전역적으로 퍼져있네요!
여러 곳에서 사용하는 것이 아니라면 분산되어 있는 것이 가독성을 저하시킨다고 생각합니다!

동의합니다. 한 곳에 모았습니다~!

SeongHoonC · 2023-09-21T00:29:24Z

android-di/src/main/java/io/hyemdooly/androiddi/base/HyemdoolyActivity.kt

+ }
+ override fun onCreate(savedInstanceState: Bundle?) {
+ super.onCreate(savedInstanceState)
+ hyemdoolyViewModel.activityModule.injectFields(this)


ActivityModule 이 activityContext 를 가진다.

ActivityModule 을 ViewModelModule 이 가지고 있다.

Activity 가 재생성 될 때 다음 Context 로 갈아끼워주긴 하지만 메모리 릭이 발생할 수 있는 상태라고 판단됩니다.

ActivityModule이 ViewModel 사이클보다 짧으니 ViewModel이 가지고 있는 것이 자연스럽지 못하다는 생각이 들긴 합니다!
하지만 Activity가 재생성될 때 다음 Context로 다시 끼워줄텐데, 메모리릭이 발생하는 때가 언제일까요..?

저도 딱히 없을거라 생각하는데 재생성할 때 갈아끼우면 Activity 의 onDestroy 가 호출되어도 context 를 참조하고 있어서 파괴되지 못하는 순간이 있을 것 같아요!

따라서 그 전에 정리하는게 좋다고 생각했습니다!

오 그럴 수도 있겠네요!!! 감사합니당 베르👍

SeongHoonC · 2023-09-21T00:30:46Z

android-di/src/main/java/io/hyemdooly/androiddi/module/ModuleFactory.kt

+ fun createApplicationModule(applicationContext: Context) =
+ modules.applicationModule(applicationContext)
+
+ fun createViewModelWithApplicationModule(


변수명을 통해 무엇을 필요로 하는지 알 수 있을 것 같네요!

createViewModelOf() or createViewModelWith()

등으로 수정할 수 있지 않을까요?

Of로 수정했습니다!

SeongHoonC · 2023-09-21T00:38:41Z

di/src/main/java/io/hyemdooly/di/Module.kt

+ fun <T : Any> injectFields(instance: T) {
+ val properties =
+ instance::class.declaredMemberProperties.filter { it.hasAnnotation<Inject>() }
+
+ properties.forEach { property ->
+ property.isAccessible = true
+ property.javaField?.let {
+ val type = it.type.kotlin
+ val fieldValue = getInstance(type)
+ it.set(instance, fieldValue)
+ }
+ }
+ }
+}


public 으로 열린 함수에 대해서는 테스트가 필요할 것으로 생각됩니다ㅎㅎ

SeongHoonC

둘리!

리뷰 반영 확인 완료했습니다!
질문 답변이 하나 있는데 위 코멘트 확인해주세요!

1,2,3,4단계 동안 자동 DI 만들기 힘들었을텐데 고생많으셨어요!

점점 좋은 구조로 변경되는게 보여서 리뷰하면서 많이 배웠습니다!
남은 미션도 화이팅!

SeongHoonC · 2023-09-25T07:39:30Z

di/src/test/kotlin/io/hyemdooly/di/ModuleTest.kt

+ @Test
+ fun `type과 일치하는 인스턴스가 없을 경우 type의 자식 클래스 객체를 리턴할 수 있다`() {


테스트 추가 굿

hyemdooly added 8 commits September 18, 2023 17:42

refactor: Container+Injector 합치기

251ddfa

refactor: Module에 모든 기능 포함

c5e3fa1

test: 리팩터링한 Module test 작성

49c9795

feat: androdi di 작성

16b9faf

test: androdi di 테스트 조건 추가

f2e8530

feat: android di 적용

a3f7166

refactor: 안쓰는 프로퍼티 삭제

84fe1b9

refactor: provider가 싱글톤 아니어야하는 경우 추가

1783920

SeongHoonC requested changes Sep 21, 2023

View reviewed changes

SeongHoonC reviewed Sep 21, 2023

View reviewed changes

hyemdooly added 10 commits September 24, 2023 16:04

refactor: 모듈 test 네이밍 refactor

8bbb45c

refactor: 애플리케이션 모듈 생성 함수 네이밍 변경

982e591

refactor: 뷰모델 모듈 생성 함수 네이밍 변경

9d059fb

refactor: 테스트용 클래스들 한곳에 모음

3c86a56

refactor: 필요없는 함수 삭제

c717f9d

test: 테스트 추가

07252cc

refactor: 테스트 변경

1d986ef

refactor: 테스트 추가

fc3842e

refactor: ViewModel에서 Activity Module 분리

a76cfc7

refactor: 필요없는 주석삭제

ba98f0f

SeongHoonC approved these changes Sep 25, 2023

View reviewed changes

SeongHoonC merged commit 0364c64 into woowacourse:hyemdooly Sep 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[둘리] 4단계 자동 DI 미션 제출합니다 #57

[둘리] 4단계 자동 DI 미션 제출합니다 #57

hyemdooly commented Sep 18, 2023 •

edited

Loading

SeongHoonC left a comment

SeongHoonC Sep 20, 2023

SeongHoonC Sep 20, 2023

SeongHoonC Sep 20, 2023

hyemdooly Sep 24, 2023

SeongHoonC Sep 20, 2023

SeongHoonC Sep 21, 2023

hyemdooly Sep 24, 2023

SeongHoonC Sep 21, 2023

hyemdooly Sep 24, 2023

SeongHoonC Sep 21, 2023

hyemdooly Sep 24, 2023

SeongHoonC Sep 21, 2023

hyemdooly Sep 24, 2023

SeongHoonC Sep 21, 2023

hyemdooly Sep 24, 2023

SeongHoonC Sep 25, 2023

hyemdooly Sep 25, 2023

SeongHoonC Sep 21, 2023

hyemdooly Sep 24, 2023

SeongHoonC Sep 21, 2023

SeongHoonC left a comment •

edited

Loading

SeongHoonC Sep 25, 2023

		val key = instances.keys.firstOrNull { it.isSubclassOf(type) }
		if (key != null && instances[key] != null) return instances[key]

		val isInitailzeModule: Boolean
		get() = ::activityModule.isInitialized && ::viewModelModule.isInitialized

		@Test
		fun `type과 일치하는 인스턴스가 없을 경우 type의 자식 클래스 객체를 리턴할 수 있다`() {

[둘리] 4단계 자동 DI 미션 제출합니다 #57

[둘리] 4단계 자동 DI 미션 제출합니다 #57

Conversation

hyemdooly commented Sep 18, 2023 • edited Loading

di

android-di

app

추가로 고민인 부분들

SeongHoonC left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SeongHoonC left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hyemdooly commented Sep 18, 2023 •

edited

Loading

SeongHoonC left a comment •

edited

Loading