[산군] 1단계 자동 DI 미션 제출합니다#24
Conversation
EmilyCh0
left a comment
There was a problem hiding this comment.
1단계 미션 고생하셨습니다 👍
산군이 PR에 남긴 고민과 개인적으로 궁금한 것들 코멘트에 남겼어요! 확인해주시고 다시 리뷰 요청해주세요!
| object DependencyInjector { | ||
| lateinit var repositoryDependency: RepositoryDependency | ||
|
|
||
| inline fun <reified T : ViewModel> inject(): ViewModelProvider.Factory { |
There was a problem hiding this comment.
DependencyInjector의 inject() 메서드가 뷰모델 팩토리를 리턴하는게 좀 어색한 것 같아요!
하나의 함수로 모든 인스턴스를 인-아웃할 수 있어야하나?
1.1. 뷰모델만을 위한 inject, 레포지토리만을 위한 inject 다 나뉘어야하나?
inject가 단순히 primaryConstructor와 의존성 주입에 필요한 인스턴스들을 사용해 필요한 인스턴스를 생성해주는 함수라면 굳이 나눌 필요가 있나요? 함수를 따로 두어 얻을 수 있는 이점이 무엇일까요?
There was a problem hiding this comment.
들어온 제네릭타입에 맞게 인스턴스를 반환하는 구조로 리팩터링했습니다!
| repositoryDependency::class.declaredMemberProperties.find { | ||
| it.returnType.jvmErasure == paramType | ||
| }?.getter?.call(repositoryDependency) |
There was a problem hiding this comment.
repositoryDependecy에서 찾을 수 없는 경우에는 뷰모델을 생성할 수 없을테니 예외를 던져도 좋을 것 같아요!
There was a problem hiding this comment.
현재 find로 컨테이너의 내부프로퍼티를 리플렉션해서 인스턴스를 만들고 있는데, list를 순회하는 것 보다 비효율적이라고 생각합니다.
2.1. 수업 때 배운것처럼, application 클래스에 인스턴스들을 직접 add해줘야할까?
매번 repositoryDependency의 declaredMemberProperties를 돌면서 찾는 것이 아니라 Injector 초기화 시 repositoryDependency를 돌면서 인스턴스를 모두 Map에 저장해두고 사용하는 건 어떤가요?
| return viewModelFactory { | ||
| initializer { | ||
| clazz.primaryConstructor!!.call(*instance.toTypedArray()) | ||
| } | ||
| } |
There was a problem hiding this comment.
inject()가 호출될 때마다 매번 특정 뷰모델을 생성하는 팩토리를 만드는 형태인 것 같아요.
뷰모델 팩토리가 뷰모델을 생성하는 하나의 공통 로직을 사용한다면 모든 뷰모델이 같은 팩토리를 사용할 수 있지않을까요?
There was a problem hiding this comment.
- 주생성자는 왜 nullable하며, 해당 사항을 마주칠때가 언제일까?
3.1. 인터페이스는 기본생성자가 없을텐데, 왜 지금 널처리를 해주지않았는데 잘 작동하는가..
테스트해보니까 interface나 object의 primaryConstructor가 null이네요! 저도 primaryConstructor가 어떤 경우에 null인지는 생각해본적이 없었는데 새로 알아갑니다 😄
There was a problem hiding this comment.
하나의 팩토리 메서드를 사용하도록 바꾸어주었습니다!
가능하면 뷰모델 프로바이더도 합치고싶었는데 이부분은 조금 어려워서 미루겠습니다 ㅎ
| interface RepositoryDependency { | ||
| val cartRepository: CartRepository | ||
| val productRepository: ProductRepository | ||
| } |
There was a problem hiding this comment.
RepositoryDependency를 추상화한 이유가 있나요?
| class ShoppingApplication : Application() { | ||
| override fun onCreate() { | ||
| super.onCreate() | ||
| DependencyInjector.repositoryDependency = RepositoryDependencyContainer() |
There was a problem hiding this comment.
컨테이너는 인스턴스를 가지고 있는 로케이터 주체가되며, 해당 로케이터에 주입해주는 위치는 앱이 시작되는 EntryPoint인 어플리케이션 클래스가 맞다고 생각해서 해당부분에서 초기화 해주고 있습니다!
지금은 DSL을 이용한 방식으로 바뀌었지만, 초기화 위치는 기존과 동일합니다
EmilyCh0
left a comment
There was a problem hiding this comment.
1단계 미션 고생하셨습니다 👍 2,3단계 진행을 위해 우선 머지하고 코멘트는 2,3단계 리뷰에 함께 남길게요! 다음 미션도 화이팅!
안녕하세요 해선생님
미션을 어제 부랴부랴 시작해서 최소 구현사항만 구현해버렸읍니다.
구현하면서 여러가지 고민들이 머릿속에 마구마구 생겼는데 해시 생각도 궁금합니다.
1.1. 뷰모델만을 위한 inject, 레포지토리만을 위한 inject 다 나뉘어야하나?
2.1. 수업 때 배운것처럼, application 클래스에 인스턴스들을 직접 add해줘야할까?
3.1. 인터페이스는 기본생성자가 없을텐데, 왜 지금 널처리를 해주지않았는데 잘 작동하는가..
테스트 및 위 고민사항들은 남은기간동안 열심히 짱구굴리면서 리팩터링 해보겠습니다 화이팅