-
Notifications
You must be signed in to change notification settings - Fork 0
[Refactor] savedstatehandle 분리 #227 #230
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
base: develop
Are you sure you want to change the base?
Conversation
- savedStateHandle 기반 네비게이션 결과 처리 로직을 HomeViewModel로 이동 - Screen은 ViewModel 상태만 관찰 - 어르신 이름 변경 결과를 ViewModel에서 원샷 이벤트로 관리
- 화면에서는 savedStateHandle을 감지해 ViewModel에 이벤트만 전달
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Walkthrough화면에서 SavedStateHandle 의존성을 제거하고, Home/Statistics 화면의 navigation result 처리를 ViewModel의 상태/콜백으로 이동한 리팩토링입니다. (뷰모델로부터 StateFlow 구독 및 ViewModel 메서드 호출 방식으로 변경) Changes
Sequence Diagram(s)(생성 기준 미충족 — 생략) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeViewModel.kt`:
- Around line 44-45: HomeViewModel 클래스의 소스 내 연속된 빈 줄(NoConsecutiveBlankLines)
위반을 해결하려면 HomeViewModel.kt 파일(클래스 정의 주변, 대략 44-45행 부근)에서 연속으로 들어가 있는 빈 줄 하나를
삭제하여 빈 줄이 연속되지 않게 정리하세요; 간단히 불필요한 빈 줄을 제거하면 Detekt 경고가 해소됩니다.
🧹 Nitpick comments (3)
app/src/main/java/com/konkuk/medicarecall/ui/feature/statistics/viewmodel/StatisticsViewModel.kt (1)
32-35:refresh()호출 시 60초 쓰로틀링으로 인해 갱신이 무시될 수 있어요.
refresh()함수(line 76)에currentTime - lastFetchTime < 60000쓰로틀링이 적용되어 있어서, 사용자가 복약 정보를 수정한 뒤 60초 이내에 이 화면으로 돌아오면 변경 사항이 반영되지 않을 수 있어요.복약 변경은 명시적인 사용자 액션이므로, 쓰로틀링을 우회하는 방식을 고려해 보시면 좋겠어요.
♻️ 제안하는 수정
// 네비게이션 결과(복약 변경 등)를 ViewModel 책임으로 처리 fun onMedsChanged() { - refresh() + val id = _selectedElderId.value ?: return + val start = _currentWeek.value.first + getWeeklyStatistics( + elderId = id, + startDate = start, + ignoreLoadingGate = true, + ) }app/src/main/java/com/konkuk/medicarecall/ui/feature/statistics/screen/StatisticsScreen.kt (1)
108-121:currentBackStackEntry참조를 캐싱하여 일관성을 확보하세요.
navController.currentBackStackEntry를 두 번 호출(line 110, 116)하고 있는데,collect블록 실행 시점에 backStackEntry가 변경될 가능성이 있어요. 참조를 한 번만 가져와서 재사용하는 것이 안전합니다.♻️ 제안하는 수정
// 복약 변경 이벤트 수신 LaunchedEffect(Unit) { - navController.currentBackStackEntry - ?.savedStateHandle + val savedStateHandle = navController.currentBackStackEntry?.savedStateHandle + savedStateHandle ?.getStateFlow("medsChanged", false) ?.collect { changed -> if (changed) { statisticsViewModel.onMedsChanged() - navController.currentBackStackEntry - ?.savedStateHandle - ?.set("medsChanged", false) + savedStateHandle["medsChanged"] = false } } }app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeViewModel.kt (1)
36-43: 문자열 상수 추출 권장
"ELDER_NAME_UPDATED"키가 두 곳에서 사용되고 있습니다. 기존KEY_SELECTED_ELDER_ID패턴처럼 companion object에 상수로 추출하면 오타 방지 및 유지보수에 유리합니다.♻️ 리팩토링 제안
private companion object { const val TAG = "HomeViewModel" const val KEY_SELECTED_ELDER_ID = "selectedElderId" + const val KEY_ELDER_NAME_UPDATED = "ELDER_NAME_UPDATED" }그리고 사용하는 곳에서:
private val _updatedName: StateFlow<String?> = - savedStateHandle.getStateFlow("ELDER_NAME_UPDATED", null) + savedStateHandle.getStateFlow(KEY_ELDER_NAME_UPDATED, null) fun clearUpdatedName() { - savedStateHandle.remove<String>("ELDER_NAME_UPDATED") + savedStateHandle.remove<String>(KEY_ELDER_NAME_UPDATED) }
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeViewModel.kt
Outdated
Show resolved
Hide resolved
ikseong00
left a comment
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.
type safe 를 사용하고 있어서 savedStateHandle.toRoute() 로 안정화된 타입을 주고받을 수 있습니다1
|
|
||
| // 이름 업데이트 수신 | ||
| private val _updatedName: StateFlow<String?> = | ||
| savedStateHandle.getStateFlow("ELDER_NAME_UPDATED", null) |
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.
혹시 이 부분을 어디서 넘겨주고 있나요??
🔗 관련 이슈
📙 작업 설명
💬 추가 설명 or 리뷰 포인트 (선택)
Summary by CodeRabbit
릴리즈 노트
✏️ Tip: You can customize this high-level summary in your review settings.