[산군] 4단계 자동 DI 미션 제출합니다#64
Conversation
EmilyCh0
left a comment
There was a problem hiding this comment.
2,3 단계 요구사항에 테스트까지 추가하느라 할 게 많았을 텐데 미션 너무 고생하셨습니다!! 코멘트 확인해 주시고, 남은 요구사항 완료해서 다시 리뷰 요청해주세요!
| import com.created.customdi.annotation.Qualifier | ||
|
|
||
| @Qualifier | ||
| annotation class Database |
There was a problem hiding this comment.
한정자는 구현체를 식별할 수 있는 일종의 ID라고 생각하는데요!
계층 구조로 같은 한정자를 사용한다면 Qualifier의 의미가 조금 퇴색된다고 생각합니다!
해시가 달아준 코맨트가 제가 이해한 바가 맞을까요?
There was a problem hiding this comment.
식별에 사용하는 일종의 ID로 CartRespository에서만 사용하는 Database 어노테이션이라면
CartDatabase 이런식으로 식별 가능한 이름이어야 하는 것 아닐까해서 남긴 코멘트였습니다!
| implementation("org.jetbrains.kotlin:kotlin-reflect:1.8.21") | ||
| implementation("androidx.core:core-ktx:1.9.0") | ||
| implementation("androidx.appcompat:appcompat:1.6.1") | ||
| implementation("com.google.android.material:material:1.9.0") |
| import kotlin.reflect.jvm.jvmErasure | ||
|
|
||
| object Injector { | ||
| const val INVALID_CLASS_TYPE = "[ERROR] Its class type cannot be injected" |
There was a problem hiding this comment.
inline내부에서만 사용되는 문자열이라 private이 안되는데 방법이 있을까유
There was a problem hiding this comment.
앗 이 부분은 제가 잘못 본 것 같아요 😅 public하게 둘수밖에 없네요!
| val module = modules.find { module -> | ||
| module::class.declaredFunctions.any { | ||
| if (func.annotations.any { it.hasQualifier() }) { | ||
| it.annotations.filter { it.hasQualifier() } == func.annotations.filter { it.hasQualifier() } | ||
| } else { | ||
| it.returnType.jvmErasure == func.returnType.jvmErasure | ||
| } | ||
| } | ||
| } ?: throw IllegalArgumentException(INVALID_FUNCTION) |
There was a problem hiding this comment.
코드를 이해하기 너무 어려워요! it 보다는 이름을 정해주는게 어떨까요?
| @Test | ||
| fun `생성자로 QualifiedClass3을 갖는 A 인스턴스가 생성된다`() { | ||
| // given : QualifiedClass3는 Qualifier를 가지고 있다 | ||
| class A(@TestQualifier3 qualifiedClass3: QualifiedClass3) | ||
|
|
||
| // when : inject를 호출하면 | ||
| val actual = Injector.inject<A>() | ||
|
|
||
| // then : 해당 타입에 대한 인스턴스가 생성된다. | ||
| assertEquals(true, actual is A) | ||
| } |
There was a problem hiding this comment.
Qualifier를 테스트하려면 클래스 A가 Qualify 타입을 받아야 할 것 같아요!
class A(@TestQualifier3 qualifiedClass3: Qualify)생성된 actual 인스턴스가 가지고 있는 qualifiedClass3가 진짜로 QualifiedClass3 타입인지도 확인하면 좋을 것 같습니다!
| import org.junit.Test | ||
| import kotlin.reflect.full.primaryConstructor | ||
|
|
||
| class InjectorTest { |
There was a problem hiding this comment.
테스트 굿 👍
Single 인스턴스를 두 번 생성했을 때 정말 싱글톤인지, Normal을 두 번 생성했을 때 다른 인스턴스인지도 테스트해보면 좋을 것 같아요!
There was a problem hiding this comment.
dateFormatter도 필드 인젝션으로 주입해 볼 수 있을 것 같아요!
| Log.d("12312311", property.toString()) | ||
| Log.d("12312322", property.parameters.toString()) | ||
| val instance = property.getSingletonIfInstantiated() | ||
| ?: property.instantiate() | ||
| Log.d("12312344", property.toString()) |
| @InMemory | ||
| fun provideCartRepository(): CartRepository = | ||
| InMemoryCartRepository() | ||
|
|
||
| @Database | ||
| fun provideCartRepository(cartProductDao: CartProductDao): CartRepository = | ||
| DatabaseCartRepository(cartProductDao) |
There was a problem hiding this comment.
- CartRepository는 앱 전체 LifeCycle 동안 유지되도록 구현한다.
CartRepository는 Singleton 어노테이션을 붙여야 하는 거 맞나요??
| fun Any.getSingletonIfInstantiated(): Any? { | ||
| val type = when (this) { | ||
| is KProperty1<*, *> -> returnType.jvmErasure | ||
| is KParameter -> type.jvmErasure | ||
| else -> throw IllegalArgumentException() | ||
| } | ||
|
|
||
| return DiContainer.singletonInstance[type] | ||
| } |
There was a problem hiding this comment.
예외상황을 다 찾지는 못했는데.. 일단 싱글톤인데 Qualifier가 붙은 경우 인스턴스를 못 찾는 것 같아요! (CartRepository가 매번 생성됨)
|
흐에... 미션..이 우선순위가 많이 내려가서 플젝하고 근로하고 이것저것하느라 많이 못했네요.. 해시도 그동안 리뷰하느라 고생했습니다 감사함다!
혹시 궁금하시면 구경..해보셔도 되고 피알도 언제든 환영입니다! |
EmilyCh0
left a comment
There was a problem hiding this comment.
산군! DI 미션 너무 고생하셨습니다!! 더 수정하고 싶다고 하셔서 대기하고 있었는데 리뷰를 더 미룰 수 없을 것 같아 우선 approve 하겠습니다! 머지 원하실 때 dm 주세요~
| private fun setupDateFormatter() { | ||
| dateFormatter = DateFormatter(this) | ||
| } |
There was a problem hiding this comment.
여전히 개발자가 직접 DateFormatter 인스턴스를 관리하는 것으로 보이네요! 저는 다음과 같이 액티비티에 필드 주입을 했는데 참고만해주세요!
→ CartActivity가 DiActivity 상속, DiActivity가 AppCompatActivity 상속하도록 하고 DiActivity의 onCreate에서 필드 주입. (약간 BaseActivity 느낌으로??)
안녕하세요 해선생님.. 주변에서 해시 짱잘했다는 소문이 자자자합니다..
구조를 다 고쳤는데 크게 달라진건 없는 것 같고 해시랑 구조 비슷해보이긴해요
테스트를 짜면서 하긴했는데, 아직 테린이라 테스트 많이 봐주시면 감사하겠습니다.
테스트맞추면서 코드짤때는 깔끔하게 짰는데, 테스트를 구리게짜서 그런가 실제 본 코드에 도입할 때는 엄청 예외상황이 많더라구요.. 시간이없어서 예외상황 급하게 처리하다보니 Injector가 굉장히 비대해졌습니다..
현재 테스트와 빌드는 터지진않습니다 잘부탁합니다!