[REFACTOR] 모듈간의 의존성을 깔끔하게 정리해보자#223
Merged
Merged
Conversation
MoonsuKang
approved these changes
Jun 23, 2025
Member
MoonsuKang
left a comment
MoonsuKang
left a comment
There was a problem hiding this comment.
수고하셨습니다!!
저희 코어모듈이 10개라 그래프가 이쁘게 그려지지 않는건 쩔수 인거 같습니다 ㅠㅠㅠ
|
|
||
| @Inject | ||
| lateinit var userPreferences: UserPreferences | ||
| lateinit var userDataRepository: UserDataRepository |
Comment on lines
+5
to
+27
| interface UserLocalDataSource { | ||
| val userIdFlow: Flow<Long?> | ||
| val userNameFlow: Flow<String?> | ||
| val onboardingCompletedFlow: Flow<Boolean> | ||
| val fortuneIdFlow: Flow<Long?> | ||
| val fortuneDateFlow: Flow<String?> | ||
| val fortuneImageIdFlow: Flow<Int?> | ||
| val fortuneScoreFlow: Flow<Int?> | ||
| val hasNewFortuneFlow: Flow<Boolean> | ||
| val firstDismissedAlarmIdFlow: Flow<Long?> | ||
|
|
||
| suspend fun saveUserId(userId: Long) | ||
| suspend fun saveUserName(userName: String) | ||
| suspend fun setOnboardingCompleted() | ||
| suspend fun saveFortuneId(fortuneId: Long) | ||
| suspend fun markFortuneAsChecked() | ||
| suspend fun saveFortuneImageId(imageResId: Int) | ||
| suspend fun saveFortuneScore(score: Int) | ||
| suspend fun saveFirstDismissedAlarmId(alarmId: Long) | ||
| suspend fun clearDismissedAlarmId() | ||
| suspend fun clearUserData() | ||
| suspend fun clearFortuneId() | ||
| } |
Member
There was a problem hiding this comment.
P4
지금 UserLocalDataSource에 사용자, 운세, 알람 관련 데이터가 함께 들어가 있는데요,
책임 관점에서 분리하면 유지보수나 테스트 코드 작성 시 더 유리할 것 같아요!
User, Fortune, 이렇게? 그리고 firstDismissedAlarmIdFlow 요건 지금 alrarmLocalDatasource로 넣어도 될 것 같다는 생각!
Member
Author
There was a problem hiding this comment.
UserLocalDataSource를 관심사별로 분리했습니당
나누다보니 기존에 local과 remote 각각에 별도의 repository 구현체를 두었다보니, 같은 클래스가 중복되는 문제가 생겼고....
Repository는 도메인 단위로 관리되기 때문에 굳이 데이터 출처로 나누는 것이 필요없다고 생각해서 비슷한 역할을 하는 구현체끼리 묶어서 통합했습니당
Comment on lines
+5
to
+8
| interface AlarmScheduler { | ||
| fun scheduleAlarm(alarm: Alarm) | ||
| fun unScheduleAlarm(alarm: Alarm) | ||
| } |
Member
There was a problem hiding this comment.
P5
옹 저도 스케줄러 부분은 도메인으로 빼고 PendingIntent는 코어에 두는 방향 찬성합니다잉
…toryimpl 디렉토리로 통합하여 도메인 중심 구조로 정리
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issue 🛠
closed #222
어떤 변경사항이 있었나요?
CheckPoint ✅
PR이 다음 요구 사항을 충족하는지 확인하세요.
Work Description ✏️
Uncompleted Tasks 😅
To Reviewers 📢
혼란하다 혼란해
feature:alarm-interaction에서는 BroadcastReceiver 등록을 위해 core:alarm 모듈 종속성을
feature:mission에서는 createDissmissPendingIntent() 함수 사용을 위해 core:alarm 모듈 종속성을 가지고 있는데
알람 스케쥴링과 같이 domain layer에서 인터페이스를 선언해서 하는게 좋을지... 아니면 그냥 직접적으로 core:alarm 모듈을 사용하도록 하는 것이 좋을까요?