-
Notifications
You must be signed in to change notification settings - Fork 1
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
[REFACTOR/#151] Auth 모듈 통신 관련 UseCase 패턴으로 교체 #152
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
리뷰 남겼슴다~~! @Marchbreeze
@Module | ||
@InstallIn(SingletonComponent::class) | ||
class UseCaseModule { | ||
@Singleton |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UseCase를 특별하게 Singleton으로 만든 이유가 있을까?.?
UseCase는 보통 상태를 가지지 않는 단일 책임 비즈니스 로직이고
이미 repository 단에서 single톤이기에 특별한 이유가 없다면 사용 안해도 될거같슴다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오호 ... 하나의 기능 구현이라면 싱글톤이 아닌게 오히려 덜꼬일 것 같기는 하네요 !
@InstallIn(SingletonComponent::class) | ||
class UseCaseModule { | ||
@Singleton | ||
@Provides |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yello 때는 별 뜻없이 provide를 사용하긴 했는데 외부 라이브러리 주입이 아니라면 bind로 주입하는걸 추천합니댜,
repository나 dataSource도
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
관련해서도 잔뜩 알아보고 오겠습니당
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
추가로 싱글톤 제거하고 UseCase에서 Inject로 주입한다면 UseCaseModule은 제거해도 됩니다~ @Marchbreeze
⛳️ Work Description