[Refactor/#136] 홈화면 루틴 완료 로직을 리펙토링 합니다.#137
Conversation
- Routines -> RoutineSchedule
- 루틴/서브루틴 완료 여부를 토글할 때, 기존의 isCompleted Boolean 값을 직접 전달하는 방식에서 파라미터 없이 호출하는 방식으로 변경
- 기존 RoutineUiModel에서 사용하는 프로퍼티만을 전달하도록 변경
- homeState, homeSideEffect 수정 - homeIntent 제거
- RoutineToggleState 정의 - 기존 viewmodel에서 비즈니스 로직 domain으로 변경
- 루틴 완료 로직 ToggleStrategy을 통해 main/sub 분리 - 초기화 메소드 initialize로 통합
개요루틴 관련 도메인 모델의 네이밍을 리팩토링하고(DayRoutines → DailyRoutines, Routines → RoutineSchedule), 프로퍼티명을 일관성 있게 변경(routineId → id, routineName → name 등)했으며, HomeViewModel의 아키텍처를 MVI에서 Orbit 기반 ContainerHost로 마이그레이션하고 토글 로직을 새로운 ToggleRoutineUseCase로 분리했습니다. 변경사항
시퀀스 다이어그램sequenceDiagram
actor User
participant HomeScreen as HomeScreen<br/>(UI)
participant HomeVM as HomeViewModel<br/>(Orbit)
participant ToggleUC as ToggleRoutineUseCase
participant RoutineUC as RoutineCompletionUseCase
participant Repo as Repository
User->>HomeScreen: 루틴 토글 클릭<br/>onRoutineToggle(routineId)
HomeScreen->>HomeVM: toggleRoutine(routineId)
HomeVM->>ToggleUC: toggleMainRoutine(isCompleted, subStates)
ToggleUC-->>HomeVM: RoutineToggleState
HomeVM->>HomeVM: updatePendingChanges(routineId, toggleState)
HomeVM->>RoutineUC: routineCompletionUseCase.updateRoutine(...)
RoutineUC->>Repo: 변경사항 동기화
Repo-->>RoutineUC: Result<Unit>
HomeVM->>HomeVM: container.reduce { state }
HomeVM-->>HomeScreen: 상태 업데이트
HomeScreen-->>User: UI 재렌더링
sequenceDiagram
participant OldArch as MVI 아키텍처<br/>(이전)
participant NewArch as Orbit 아키텍처<br/>(현재)
rect rgb(230, 240, 255)
Note over OldArch: HomeIntent → Reducer
OldArch->>OldArch: sendIntent(HomeIntent)
OldArch->>OldArch: reduce(state, intent)
OldArch->>OldArch: updateState()
end
rect rgb(240, 255, 240)
Note over NewArch: ViewModel Method → Container
NewArch->>NewArch: selectDate(date)
NewArch->>NewArch: container.reduce { }
NewArch->>NewArch: postSideEffect()
end
코드 리뷰 예상 난이도🎯 4 (복잡함) | ⏱️ ~60분 추가 검토 필요 영역:
시
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
domain/src/main/java/com/threegap/bitnagil/domain/routine/model/RoutineToggleState.kt (1)
3-6: 프로퍼티 네이밍을 개선하는 것을 고려해보세요.
subRoutinesIsCompleted는 문법적으로 다소 어색합니다. 다음 중 하나로 변경하는 것이 더 자연스럽습니다:
subRoutineCompletionStates(도메인 모델의 Routine.subRoutineCompletionStates와 일관성)areSubRoutinesCompletedsubRoutinesCompleted다음 diff를 적용하여 도메인 모델과 일관성을 맞출 수 있습니다:
data class RoutineToggleState( val isCompleted: Boolean, - val subRoutinesIsCompleted: List<Boolean>, + val subRoutineCompletionStates: List<Boolean>, )presentation/src/main/java/com/threegap/bitnagil/presentation/routinelist/model/DayRoutinesUiModel.kt (1)
12-15: 도메인 모델 변경사항이 정확히 반영되었습니다.
DailyRoutines로 업데이트하고routines프로퍼티를 올바르게 매핑하고 있습니다.참고: 도메인 모델은
routines, UI 모델은routineList라는 다른 프로퍼티명을 사용 중입니다. 이것이 의도적인 UI 레이어 네이밍 컨벤션이라면 문제없으나, 일관성을 위해 둘 다routines로 통일하는 것도 고려해볼 수 있습니다.domain/src/main/java/com/threegap/bitnagil/domain/emotion/usecase/FetchTodayEmotionUseCase.kt (1)
11-14: LocalDate.now() 의존성 분리 제안
이제 현재 날짜를 내부에서 직접 계산하면서 테스트나 타임존 제어가 쉽지 않아졌습니다.Clock혹은 별도 DateProvider를 주입하면 deterministic 테스트가 가능하고, 서버 기준 날짜를 맞춰야 하는 요구도 대응하기 수월합니다. 아래처럼 수정해 보면 어떨까요?-import java.time.LocalDate +import java.time.Clock +import java.time.LocalDate @@ -class FetchTodayEmotionUseCase @Inject constructor( - - private val emotionRepository: EmotionRepository, -) { - suspend operator fun invoke(): Result<TodayEmotion?> { - val currentDate = LocalDate.now().toString() - return emotionRepository.fetchTodayEmotion(currentDate) - } +class FetchTodayEmotionUseCase @Inject constructor( + private val emotionRepository: EmotionRepository, + private val clock: Clock, +) { + suspend operator fun invoke(): Result<TodayEmotion?> { + val currentDate = LocalDate.now(clock).toString() + return emotionRepository.fetchTodayEmotion(currentDate) + } }presentation/src/main/java/com/threegap/bitnagil/presentation/home/HomeViewModel.kt (1)
185-198: 초기화 및 데이터 가져오기 로직이 효율적이고 견고합니다.
coroutineScope를 사용한 병렬 초기화와 로딩 상태 추적이 올바르게 구현되었습니다.fold를 통한 에러 처리도 적절합니다.선택사항: 프로덕션 환경에서는
Log.e외에 Firebase Crashlytics 등의 에러 추적 서비스 연동을 고려해보세요.Also applies to: 249-292
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
data/src/main/java/com/threegap/bitnagil/data/routine/model/response/DayRoutinesDto.kt(2 hunks)data/src/main/java/com/threegap/bitnagil/data/routine/model/response/RoutineDto.kt(1 hunks)data/src/main/java/com/threegap/bitnagil/data/routine/model/response/RoutinesResponseDto.kt(2 hunks)data/src/main/java/com/threegap/bitnagil/data/routine/repositoryImpl/RoutineRepositoryImpl.kt(1 hunks)domain/src/main/java/com/threegap/bitnagil/domain/emotion/usecase/FetchTodayEmotionUseCase.kt(1 hunks)domain/src/main/java/com/threegap/bitnagil/domain/routine/model/DailyRoutines.kt(1 hunks)domain/src/main/java/com/threegap/bitnagil/domain/routine/model/DayRoutines.kt(0 hunks)domain/src/main/java/com/threegap/bitnagil/domain/routine/model/Routine.kt(1 hunks)domain/src/main/java/com/threegap/bitnagil/domain/routine/model/RoutineSchedule.kt(1 hunks)domain/src/main/java/com/threegap/bitnagil/domain/routine/model/RoutineToggleState.kt(1 hunks)domain/src/main/java/com/threegap/bitnagil/domain/routine/model/Routines.kt(0 hunks)domain/src/main/java/com/threegap/bitnagil/domain/routine/repository/RoutineRepository.kt(1 hunks)domain/src/main/java/com/threegap/bitnagil/domain/routine/usecase/FetchWeeklyRoutinesUseCase.kt(1 hunks)domain/src/main/java/com/threegap/bitnagil/domain/routine/usecase/ToggleRoutineUseCase.kt(1 hunks)presentation/src/main/java/com/threegap/bitnagil/presentation/home/HomeScreen.kt(6 hunks)presentation/src/main/java/com/threegap/bitnagil/presentation/home/HomeViewModel.kt(2 hunks)presentation/src/main/java/com/threegap/bitnagil/presentation/home/component/block/RoutineItem.kt(3 hunks)presentation/src/main/java/com/threegap/bitnagil/presentation/home/component/block/SubRoutinesItem.kt(3 hunks)presentation/src/main/java/com/threegap/bitnagil/presentation/home/component/template/RoutineSection.kt(3 hunks)presentation/src/main/java/com/threegap/bitnagil/presentation/home/component/template/WeeklyDatePicker.kt(4 hunks)presentation/src/main/java/com/threegap/bitnagil/presentation/home/model/DailyRoutinesUiModel.kt(1 hunks)presentation/src/main/java/com/threegap/bitnagil/presentation/home/model/DayRoutinesUiModel.kt(0 hunks)presentation/src/main/java/com/threegap/bitnagil/presentation/home/model/HomeIntent.kt(0 hunks)presentation/src/main/java/com/threegap/bitnagil/presentation/home/model/HomeSideEffect.kt(1 hunks)presentation/src/main/java/com/threegap/bitnagil/presentation/home/model/HomeState.kt(1 hunks)presentation/src/main/java/com/threegap/bitnagil/presentation/home/model/RoutineScheduleUiModel.kt(1 hunks)presentation/src/main/java/com/threegap/bitnagil/presentation/home/model/RoutineUiModel.kt(1 hunks)presentation/src/main/java/com/threegap/bitnagil/presentation/home/model/RoutinesUiModel.kt(0 hunks)presentation/src/main/java/com/threegap/bitnagil/presentation/home/model/ToggleStrategy.kt(1 hunks)presentation/src/main/java/com/threegap/bitnagil/presentation/routinelist/RoutineListViewModel.kt(1 hunks)presentation/src/main/java/com/threegap/bitnagil/presentation/routinelist/model/DayRoutinesUiModel.kt(1 hunks)presentation/src/main/java/com/threegap/bitnagil/presentation/routinelist/model/RoutineUiModel.kt(1 hunks)presentation/src/main/java/com/threegap/bitnagil/presentation/routinelist/model/RoutinesUiModel.kt(1 hunks)presentation/src/main/java/com/threegap/bitnagil/presentation/writeroutine/WriteRoutineViewModel.kt(1 hunks)
💤 Files with no reviewable changes (5)
- presentation/src/main/java/com/threegap/bitnagil/presentation/home/model/RoutinesUiModel.kt
- domain/src/main/java/com/threegap/bitnagil/domain/routine/model/DayRoutines.kt
- presentation/src/main/java/com/threegap/bitnagil/presentation/home/model/DayRoutinesUiModel.kt
- domain/src/main/java/com/threegap/bitnagil/domain/routine/model/Routines.kt
- presentation/src/main/java/com/threegap/bitnagil/presentation/home/model/HomeIntent.kt
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-13T09:06:19.028Z
Learnt from: wjdrjs00
Repo: YAPP-Github/Bitnagil-Android PR: 101
File: presentation/src/main/java/com/threegap/bitnagil/presentation/setting/SettingViewModel.kt:61-67
Timestamp: 2025-08-13T09:06:19.028Z
Learning: In Android ViewModels, when logout success triggers navigation to a different screen (like NavigateToLogin), the current ViewModel's lifecycle ends, so loading states don't need to be explicitly reset in the success case since the ViewModel will be destroyed.
Applied to files:
presentation/src/main/java/com/threegap/bitnagil/presentation/home/HomeScreen.kt
🧬 Code graph analysis (4)
presentation/src/main/java/com/threegap/bitnagil/presentation/home/component/block/RoutineItem.kt (2)
core/designsystem/src/main/java/com/threegap/bitnagil/designsystem/component/atom/BitnagilIcon.kt (1)
BitnagilIcon(21-33)presentation/src/main/java/com/threegap/bitnagil/presentation/home/component/block/SubRoutinesItem.kt (1)
SubRoutinesItem(19-58)
presentation/src/main/java/com/threegap/bitnagil/presentation/routinelist/RoutineListViewModel.kt (1)
presentation/src/main/java/com/threegap/bitnagil/presentation/home/HomeViewModel.kt (1)
fetchWeeklyRoutinesUseCase(35-310)
presentation/src/main/java/com/threegap/bitnagil/presentation/home/HomeScreen.kt (2)
presentation/src/main/java/com/threegap/bitnagil/presentation/home/HomeViewModel.kt (4)
navigateToGuide(160-164)navigateToRegisterRoutine(172-176)navigateToEmotion(166-170)navigateToRoutineList(178-183)presentation/src/main/java/com/threegap/bitnagil/presentation/home/component/template/RoutineSection.kt (1)
RoutineSection(18-46)
presentation/src/main/java/com/threegap/bitnagil/presentation/home/HomeViewModel.kt (7)
data/src/main/java/com/threegap/bitnagil/data/user/service/UserService.kt (2)
fetchUserProfile(7-10)fetchUserProfile(8-9)domain/src/main/java/com/threegap/bitnagil/domain/user/repository/UserRepository.kt (2)
fetchUserProfile(5-7)fetchUserProfile(6-6)data/src/main/java/com/threegap/bitnagil/data/emotion/service/EmotionService.kt (1)
fetchTodayEmotion(22-25)domain/src/main/java/com/threegap/bitnagil/domain/emotion/repository/EmotionRepository.kt (1)
fetchTodayEmotion(12-12)data/src/main/java/com/threegap/bitnagil/data/routine/repositoryImpl/RoutineRepositoryImpl.kt (1)
fetchWeeklyRoutines(15-17)domain/src/main/java/com/threegap/bitnagil/domain/routine/repository/RoutineRepository.kt (2)
fetchWeeklyRoutines(7-13)fetchWeeklyRoutines(8-8)presentation/src/main/java/com/threegap/bitnagil/presentation/recommendroutine/RecommendRoutineViewModel.kt (1)
observeEmotionChangeEvent(96-102)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (33)
presentation/src/main/java/com/threegap/bitnagil/presentation/home/model/ToggleStrategy.kt (2)
3-6: 타입 안전한 토글 전략 설계 잘 구현되었습니다.sealed interface를 활용한 설계가 메인/서브 루틴 토글을 타입 안전하게 표현하고 있습니다. Main은 data object로, Sub는 index를 가진 data class로 구분하여 컴파일 타임에 유효하지 않은 상태를 방지하는 좋은 접근입니다.
5-5: 검증 완료: index 파라미터 경계 검증이 이미 구현되어 있습니다.도메인 레이어의
ToggleRoutineUseCase.toggleSubRoutine()에서if (index !in subRoutineStates.indices)체크로 인덱스 경계를 이미 검증하고 있습니다. 범위를 벗어난 인덱스는 null을 반환하므로 IndexOutOfBoundsException이 발생하지 않습니다. ToggleStrategy.Sub는 데이터 클래스이고, 실제 검증은 이를 사용하는 use case에서 수행되므로 추가 검증이 불필요합니다.Likely an incorrect or invalid review comment.
presentation/src/main/java/com/threegap/bitnagil/presentation/writeroutine/WriteRoutineViewModel.kt (1)
86-87: 도메인 모델 필드명 변경이 정확히 반영되었습니다.
routine.name과routine.repeatDays로 업데이트하여 도메인 모델의 네이밍 변경사항과 일관성을 유지하고 있습니다.presentation/src/main/java/com/threegap/bitnagil/presentation/home/model/HomeSideEffect.kt (1)
3-8: Orbit-Mvi 활용 극대화에 맞춰 깔끔하게 리팩토링되었습니다.
MviSideEffect의존성을 제거하고 필요한NavigateToRoutineList케이스를 추가하여 코드가 더 간결해졌습니다.presentation/src/main/java/com/threegap/bitnagil/presentation/routinelist/RoutineListViewModel.kt (1)
132-132: API 시그니처가 더 직관적으로 개선되었습니다.시작/종료 날짜를 별도로 전달하는 대신
currentWeek컬렉션을 직접 전달하여 호출이 더 명확하고 간결해졌습니다.presentation/src/main/java/com/threegap/bitnagil/presentation/routinelist/model/RoutineUiModel.kt (1)
32-44: 도메인 모델 필드명 변경이 일관되게 반영되었습니다.
id,name,repeatDays,isDeleted로 매핑하여 도메인 모델의 리네이밍과 완벽하게 일치합니다.domain/src/main/java/com/threegap/bitnagil/domain/routine/model/DailyRoutines.kt (1)
1-13: 명확한 도메인 모델과 우수한 문서화.KDoc을 통해
DailyRoutines의 역할과 각 프로퍼티의 의미가 명확히 설명되어 있으며, RoutineSchedule과의 관계도 잘 표현되어 있습니다.domain/src/main/java/com/threegap/bitnagil/domain/routine/model/RoutineSchedule.kt (1)
1-11: 명확하고 의미 있는 도메인 모델 네이밍.
Routines에서RoutineSchedule로 변경하여 Map 구조(날짜 → 일간 루틴)의 의도를 더 명확하게 표현하고 있습니다. KDoc도 상세하고 유용합니다.presentation/src/main/java/com/threegap/bitnagil/presentation/routinelist/model/RoutinesUiModel.kt (1)
12-16: LGTM!도메인 모델 변경에 맞춰 extension receiver와 프로퍼티 접근이 올바르게 업데이트되었습니다.
presentation/src/main/java/com/threegap/bitnagil/presentation/home/model/DailyRoutinesUiModel.kt (1)
1-17: LGTM!도메인 모델
DailyRoutines에 대응하는 UI 모델이 깔끔하게 구현되었습니다. Parcelable 구현과 toUiModel 확장 함수가 기존 패턴과 일관성 있게 작성되었습니다.data/src/main/java/com/threegap/bitnagil/data/routine/model/response/RoutineDto.kt (1)
37-51: LGTM!도메인 모델의 필드 네이밍 변경사항이 정확하게 반영되었습니다. 더 직관적인 이름으로 개선되었습니다.
data/src/main/java/com/threegap/bitnagil/data/routine/repositoryImpl/RoutineRepositoryImpl.kt (1)
15-17: LGTM!반환 타입이
RoutineSchedule로 올바르게 업데이트되었으며, 기존 로직은 그대로 유지되었습니다.data/src/main/java/com/threegap/bitnagil/data/routine/model/response/RoutinesResponseDto.kt (1)
13-18: LGTM!
RoutineSchedule도메인 모델로의 변환이 올바르게 구현되었습니다. 프로퍼티 이름이dailyRoutines로 변경되어 의도가 더 명확해졌습니다.data/src/main/java/com/threegap/bitnagil/data/routine/model/response/DayRoutinesDto.kt (1)
15-19: LGTM!
DailyRoutines도메인 모델로의 변환이 정확하게 구현되었습니다. 필드 네이밍이 일관성 있게 개선되었습니다.presentation/src/main/java/com/threegap/bitnagil/presentation/home/component/template/WeeklyDatePicker.kt (1)
58-62: 추가 검증을 통해 날짜 키 형식 일관성이 확인되었습니다.코드베이스 전체에서 다음 사항이 확인되었습니다:
- 백엔드 응답: Mock 데이터와 API 구조상 날짜는 ISO-8601 형식(yyyy-MM-dd)으로 제공됩니다 (예: "2025-08-15", "2023-10-27")
- LocalDate.toString(): Kotlin의 LocalDate 기본 toString() 메서드는 ISO-8601 형식(yyyy-MM-dd)을 반환합니다
- 데이터 흐름: 백엔드 JSON → RoutinesResponseDto (키 그대로 보존) → RoutineSchedule → date.toString()으로 접근
세 파일 모두에서 일관되게
date.toString()을 사용하며, 모든 Mock 데이터가 ISO-8601 형식을 사용하고 있으므로 날짜 키 형식이 이미 일치합니다. 추가 조치가 필요하지 않습니다.presentation/src/main/java/com/threegap/bitnagil/presentation/home/component/block/SubRoutinesItem.kt (1)
23-42: 콜백 시그니처 단순화 👍
인덱스만 넘기도록 정리하면서 UI 단의 불필요한 상태 추론이 사라져 더 깔끔해졌습니다. 상위에서 토글 상태를 일관되게 계산하기에 좋은 구조입니다.presentation/src/main/java/com/threegap/bitnagil/presentation/home/model/RoutineScheduleUiModel.kt (1)
7-17: UI 모델 변환 경로 명확화 👍
도메인 Map을 그대로 재활용하되 값만 UI 모델로 바꾸는 접근이 간결하고, 기본값을 emptyMap으로 둬서 NPE 리스크도 줄었습니다. 프리젠테이션 계층에서 다루기 편해졌네요.domain/src/main/java/com/threegap/bitnagil/domain/routine/repository/RoutineRepository.kt (1)
5-13: RoutineSchedule 중심 정렬 👍
주간 조회 반환형을 RoutineSchedule로 맞추고 일자 삭제 API를 추가한 덕분에 새 도메인 모델과 계약이 잘 정리됐습니다. 구현체들도 일관되게 따라가기 쉬울 것 같습니다.presentation/src/main/java/com/threegap/bitnagil/presentation/home/component/block/RoutineItem.kt (1)
25-94: 필드 기반 API 리팩토링 👍
UI 모델을 직접 넘기지 않고 표시/토글에 필요한 속성만 노출시키니 컴포저블 재사용성이 확실히 좋아졌습니다. 프리뷰도 명확해서 의도를 파악하기 쉽네요.presentation/src/main/java/com/threegap/bitnagil/presentation/home/component/template/RoutineSection.kt (2)
21-43: 콜백 시그니처 정리가 명확해졌습니다.
RoutineItem에 필요한 필드만 전달하도록 정리되어 UI 계층 책임이 분리되고 추후 분기 추가가 쉬워졌습니다.
51-65: 프리뷰 데이터 업데이트 확인했습니다.
신규 필드(id/name 등) 기반 샘플 구성이 최신 모델과 일치해 개발자 프리뷰 검증에 도움이 되겠습니다.domain/src/main/java/com/threegap/bitnagil/domain/routine/model/Routine.kt (1)
4-15: 도메인 모델 네이밍 정리 👍
id·name·repeatDays 등 직관적인 필드명 덕분에 상위 레이어 매핑 가독성이 좋아졌습니다.domain/src/main/java/com/threegap/bitnagil/domain/routine/usecase/ToggleRoutineUseCase.kt (1)
16-55: 토글 로직 일관성 확인했습니다.
메인 토글 시 서브 루틴 상태 일괄 반영, 서브 토글 시 전체 완료 판정 로직과 범위 체크가 명확해 요구사항과 잘 맞습니다.presentation/src/main/java/com/threegap/bitnagil/presentation/home/model/HomeState.kt (1)
7-28: HomeState 구조 개편 👍
loadingCount 기반 isLoading 계산과 RoutineScheduleUiModel 접근이 명확해져 상태 관리 흐름 파악이 쉬워졌습니다.presentation/src/main/java/com/threegap/bitnagil/presentation/home/model/RoutineUiModel.kt (1)
11-32: UI 모델 필드 네이밍이 도메인과 일치합니다.
id·name·repeatDays 등으로 정리된 덕분에 매퍼 코드가 직관적이고 중복 변환이 줄었습니다.presentation/src/main/java/com/threegap/bitnagil/presentation/home/HomeScreen.kt (3)
34-35: Orbit 마이그레이션이 올바르게 적용되었습니다.Lifecycle-aware 상태 수집에서 Orbit의
collectAsState()와collectSideEffect로의 전환이 깔끔하게 처리되었습니다.Also applies to: 46-55
156-156: LazyColumn 아이템 키 생성이 개선되었습니다.
routine.id와selectedDate를 조합하여 날짜 변경 시에도 고유한 키를 보장합니다. 이는 Compose 리컴포지션 시 아이템 재사용 문제를 방지합니다.
59-67: 콜백 구조가 명확하고 타입 안전하게 개선되었습니다.Intent 디스패칭 방식에서 ViewModel 메서드 직접 호출로 변경되어 코드가 더 직관적이고 타입 안전성이 향상되었습니다.
Also applies to: 77-78, 160-163
presentation/src/main/java/com/threegap/bitnagil/presentation/home/HomeViewModel.kt (5)
4-4: Orbit 기반 아키텍처로의 마이그레이션이 올바릅니다.
MviViewModel에서ContainerHost와ViewModel로의 전환이 정확하게 구현되었으며, Orbit의 표준 패턴을 따릅니다.Also applies to: 29-31, 45-47
49-50: 낙관적 업데이트 구조가 효과적으로 개선되었습니다.중첩 Map 구조(
Map<String, MutableMap<String, RoutineCompletionInfo>>)를 사용하여 동일 루틴의 변경 사항을 덮어쓰는 방식으로 전환했습니다. 이는 PR 설명에 명시된 대로 백업 상태 관리 없이 더 단순한 낙관적 업데이트를 구현합니다.
56-89: 날짜/주차 변경 시 데이터 손실 방지 메커니즘이 우수합니다.날짜나 주차를 변경하기 전에
syncRoutineChangesForDate를 호출하여 보류 중인 변경 사항을 플러시합니다. 이는 PR 설명에 언급된 "디바운스로 인한 요일 변경 시 누락 방지"를 효과적으로 해결합니다.
239-247: 디바운스 시간 조정이 적절합니다.2초에서 500ms로 단축하여 사용자 피드백을 개선하면서, 날짜/주차 변경 시 flush 메커니즘으로 누락을 방지합니다. 응답성과 서버 부하 사이의 균형이 적절합니다.
103-158: 루틴 상태 업데이트 로직이 정확하고 올바르게 구현되었습니다.
ToggleRoutineUseCase검증 완료:
toggleMainRoutine()→RoutineToggleState(논널 반환)toggleSubRoutine()→RoutineToggleState?(인덱스 범위 벗어나면 null 반환)Line 123의
?: return@subIntent는 필요하며 올바릅니다.toggleSubRoutine이 잘못된 인덱스에 대해 null을 반환할 수 있으므로, 호출 코드에서 elvis 연산자로 처리하는 것이 적절한 방어 로직입니다.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
domain/src/main/java/com/threegap/bitnagil/domain/routine/usecase/ToggleRoutineUseCase.kt (1)
36-52: 서브 루틴 토글 로직이 정확하나, 코드 간결화를 권장합니다.로직이 올바르게 구현되어 있습니다:
- 인덱스 검증을 통한 안전한 처리
- 특정 서브 루틴만 토글하고 나머지는 유지
isCompleted가 자동으로 모든 서브 루틴의 완료 여부를 반영다만 코드 간결화를 위해
mapIndexed를 사용하는 것을 권장합니다:fun toggleSubRoutine( index: Int, subRoutineStates: List<Boolean>, ): RoutineToggleState? { if (index !in subRoutineStates.indices) { return null } - val newState = !subRoutineStates[index] - val newSubRoutineStates = subRoutineStates.toMutableList().apply { - this[index] = newState - } + val newSubRoutineStates = subRoutineStates.mapIndexed { i, state -> + if (i == index) !state else state + } return RoutineToggleState( subRoutinesIsCompleted = newSubRoutineStates, ) }이 방식은 불변성을 더 명확하게 표현하고, 가독성도 향상됩니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
domain/src/main/java/com/threegap/bitnagil/domain/routine/model/RoutineToggleState.kt(1 hunks)domain/src/main/java/com/threegap/bitnagil/domain/routine/usecase/ToggleRoutineUseCase.kt(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
domain/src/main/java/com/threegap/bitnagil/domain/routine/model/RoutineToggleState.kt (1)
3-8: 도메인 로직 검증 필요 - 현재 구현은 기술적으로 올바릅니다스크립트 검사 결과:
newSubRoutineStates는 항상 입력subRoutineStates로부터.map()또는.toMutableList()로 생성됩니다- 빈 리스트는 입력이 비어있을 경우 발생 가능합니다
all { it }는 빈 리스트에서true를 반환하는 것이 Kotlin의 표준 동작입니다- 현재 코드에는 빈 리스트에 대한 검증이나 가드가 없습니다
기술적으로는 문제가 없지만, 도메인 관점에서 검증 필요:
- 빈 서브루틴 리스트가 발생 가능한가?
- 빈 리스트가 "완료 상태"(true)를 의미하는 것이 의도된 동작인가?
- 아니면 "정의되지 않은 상태" 또는 에러 처리가 필요한가?
참고: HomeViewModel에서는 이미 필드를
subRoutineCompletionStates로 매핑하고 있으므로, 원래 리뷰의 네이밍 제안은 프레젠테이션 계층에서 부분적으로 해결되어 있습니다.domain/src/main/java/com/threegap/bitnagil/domain/routine/usecase/ToggleRoutineUseCase.kt (1)
16-26: 메인 루틴 토글 로직의 엣지 케이스 확인 필요로직 검증 결과,
toggleMainRoutine()구현은 올바릅니다. 모든 서브 루틴을 동일한 값으로 설정하면RoutineToggleState.isCompleted게터가all { it }을 통해 정확한 값을 반환합니다.다만 엣지 케이스가 실제로 존재합니다: 코드베이스에서
subRoutineCompletionStates가 빈 리스트로 사용되는 예가 확인됩니다 (예: RoutineItem.kt의 프리뷰 함수). 이 경우all { it }는 빈 컬렉션에 대해true를 반환하므로, 서브 루틴이 없는 메인 루틴은 항상 완료 상태로 간주됩니다.확인 사항: 서브 루틴이 없는 메인 루틴이 항상 완료 상태로 간주되는 것이 의도된 동작인지 확인해주세요.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
domain/src/main/java/com/threegap/bitnagil/domain/routine/usecase/ToggleRoutineUseCase.kt (1)
37-54: 서브 루틴 토글 로직이 올바르게 구현되었습니다.인덱스 유효성 검사와 전체 완료 여부 판단 로직이 정확합니다.
현재 구현도 충분히 명확하지만, 더 함수형 스타일을 선호한다면 다음과 같이 리팩토링할 수 있습니다:
- val newState = !subRoutineStates[index] - val newSubRoutineStates = subRoutineStates.toMutableList().apply { - this[index] = newState - } + val newSubRoutineStates = subRoutineStates.mapIndexed { i, state -> + if (i == index) !state else state + }이렇게 하면 가변 리스트 생성 없이 불변 방식으로 처리할 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
domain/src/main/java/com/threegap/bitnagil/domain/routine/model/RoutineToggleState.kt(1 hunks)domain/src/main/java/com/threegap/bitnagil/domain/routine/usecase/ToggleRoutineUseCase.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- domain/src/main/java/com/threegap/bitnagil/domain/routine/model/RoutineToggleState.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
domain/src/main/java/com/threegap/bitnagil/domain/routine/usecase/ToggleRoutineUseCase.kt (2)
16-27: 잘 구현된 메인 루틴 토글 로직입니다.메인 루틴 상태를 반전시키고 모든 서브 루틴에 동일하게 적용하는 로직이 명확하고 정확합니다. 문서화도 잘 되어 있습니다.
6-54: 엣지 케이스 처리를 테스트로 검증해주세요.현재 구현은 빈 리스트와 유효하지 않은 인덱스를 올바르게 처리하지만, 다음 시나리오에 대한 단위 테스트 추가를 권장합니다:
subRoutineStates가 빈 리스트인 경우- 유효하지 않은 인덱스 (음수, 범위 초과)
- 모든 서브 루틴이 이미 완료된 상태에서 토글
- 메인 루틴을 완료한 후 하나의 서브 루틴을 미완료로 변경하는 시나리오
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
presentation/src/main/java/com/threegap/bitnagil/presentation/home/HomeViewModel.kt (1)
296-311: 동기화 실패 시 사용자 알림 추가를 고려하세요.이전 리뷰에서 언급된 것처럼, 동기화 실패 시 낙관적 업데이트를 롤백하지만 사용자에게 알림을 제공하지 않습니다. 사용자가 변경사항이 저장되지 않았음을 인지할 수 있도록 토스트나 스낵바를 통한 알림을 추가하는 것을 고려해보세요.
🧹 Nitpick comments (1)
presentation/src/main/java/com/threegap/bitnagil/presentation/home/HomeViewModel.kt (1)
103-158: 조기 반환 시 로깅을 추가하는 것을 고려하세요.Lines 106-107에서
dailySchedule이나routine을 찾지 못할 때 조용히 반환합니다. 데이터 로딩 중이거나 날짜가 범위를 벗어난 경우 정상적인 동작일 수 있지만, 디버깅과 문제 추적을 위해 로깅을 추가하는 것이 좋습니다.다음과 같이 로깅을 추가할 수 있습니다:
private suspend fun updateRoutineState(routineId: String, strategy: ToggleStrategy) { subIntent { val dateKey = state.selectedDate.toString() - val dailySchedule = state.routineSchedule.dailyRoutines[dateKey] ?: return@subIntent - val routine = dailySchedule.routines.find { it.id == routineId } ?: return@subIntent + val dailySchedule = state.routineSchedule.dailyRoutines[dateKey] ?: run { + Log.d("HomeViewModel", "dailySchedule not found for date: $dateKey") + return@subIntent + } + val routine = dailySchedule.routines.find { it.id == routineId } ?: run { + Log.d("HomeViewModel", "routine not found: $routineId") + return@subIntent + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
domain/src/main/java/com/threegap/bitnagil/domain/routine/usecase/FetchWeeklyRoutinesUseCase.kt(1 hunks)presentation/src/main/java/com/threegap/bitnagil/presentation/home/HomeViewModel.kt(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
presentation/src/main/java/com/threegap/bitnagil/presentation/home/HomeViewModel.kt (5)
domain/src/main/java/com/threegap/bitnagil/domain/user/repository/UserRepository.kt (2)
fetchUserProfile(5-7)fetchUserProfile(6-6)domain/src/main/java/com/threegap/bitnagil/domain/emotion/repository/EmotionRepository.kt (1)
fetchTodayEmotion(12-12)domain/src/main/java/com/threegap/bitnagil/domain/routine/repository/RoutineRepository.kt (2)
fetchWeeklyRoutines(7-13)fetchWeeklyRoutines(8-8)data/src/main/java/com/threegap/bitnagil/data/routine/datasource/RoutineRemoteDataSource.kt (2)
fetchWeeklyRoutines(7-13)fetchWeeklyRoutines(8-8)data/src/main/java/com/threegap/bitnagil/data/routine/datasourceImpl/RoutineRemoteDataSourceImpl.kt (1)
fetchWeeklyRoutines(15-18)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (5)
domain/src/main/java/com/threegap/bitnagil/domain/routine/usecase/FetchWeeklyRoutinesUseCase.kt (1)
3-12: 이전 논의사항이 잘 반영되었습니다.함수 시그니처를
(String, String)파라미터로 유지한 것은 이전 리뷰 논의 결과를 잘 반영한 것으로 보입니다. UseCase가 시작일/종료일만 받아 명확한 책임을 가지며, 빈 리스트 검증 문제도 해결되었습니다.RoutineSchedule로의 도메인 모델 변경도 깔끔하게 적용되었습니다.presentation/src/main/java/com/threegap/bitnagil/presentation/home/HomeViewModel.kt (4)
4-50: Orbit 아키텍처 마이그레이션이 깔끔합니다.
ContainerHost패턴으로의 전환이 잘 적용되었습니다.pendingChangesByDate구조를 통한 날짜별 변경사항 관리와routineSyncTrigger를 활용한 동기화 메커니즘이 이전 방식보다 명확합니다.
160-183: 네비게이션 메서드가 깔끔합니다.
postSideEffect를 사용한 네비게이션 처리가 Orbit 패턴에 맞게 잘 구현되었습니다.
185-198: 병렬 초기화가 효율적입니다.
coroutineScope와 여러launch블록을 활용한 병렬 데이터 로딩이 잘 구현되었습니다. 구조화된 동시성의 이점을 활용하고 있습니다.
224-247: 디바운스 시간 단축이 적절합니다.디바운스를 2초에서 500ms로 단축하고 요일 변경 시 flush 처리를 추가한 것은 PR 목표에 맞게 잘 구현되었습니다. 요일 변경 시 누락 방지가 개선되었습니다.
l5x5l
left a comment
l5x5l
left a comment
There was a problem hiding this comment.
고생 많으셨습니다 🚀
pr 병합해주시면 금일중으로 MviViewModel 제거 관련 branch 생성하겠습니다!
2 participants
[ PR Content ]
홈화면 루틴 완료 로직을 리펙토링 했습니다.
주요 변경사항 요약:
ToggleRoutineUseCase으로 비즈니스 로직을 이동시켰습니다.RoutineSection컴포넌트에 전달되고 있던 완료상태를 호이스팅 했습니다.MVIViewModel을 걷어내고 Orbit-Mvi를 극대화 하는 방향으로 수정했습니다.Related issue
Screenshot 📸
Work Description
Routines->RoutineSchedule, 해당 구조가 날짜와 당일 루틴들을 가지고 있는 Map 형식이라 단순 루틴들 보단 루틴 스케줄이 더 적합하다고 판단하여 변경했습니다.DayRoutines->DailyRoutines, 의미적으론 큰 차이가 없지만 일관성을 유지하기 위해서 변경했습니다.To Reviewers 📢
Summary by CodeRabbit
릴리스 노트
새로운 기능
리팩토링