NR-122 타이머와 힌트 화면이 viewModel을 공유해서 사용하도록 리팩토링한다. #127
Conversation
왜? 힌트 화면에서 진행률만 확인하고 힌트 차감없이 되돌아가는 기능이 새로 개발될 예정임 기존에는 힌트를 차감하고 다음 화면으로 넘어가는 방식이였으나 앞으로는 힌트 화면에서 데이터의 변경을 담당하게 될거임. 근데 이걸 setFragmentResult로 하면 다소 코드가 너저분해지고 나중에 기능이 확장될 것을 생각하면 이 방식은 누가 누구에게 무엇을 넘겼는지 파악하기 어려워짐 어떻게? argument를 넘기는 방식에서 graph viewModel을 사용해 관리하는 방법으로 변경
왜? 굉장히 긴 샘플 문자열 데이터가 들어가있었는데 이렇게 되면 ui에서 실제 데이터가 로드되기 전 초기값을 보여주게 되었을때 사용자에게 노출될 가능성이 있음 (데이터 로드전에는 로딩으로 표시한다거나.. 처리를 하면 되긴하지만 놓칠 가능성이 있으니까) 그래서 불필요한 초기값 세팅들을 수정함 id도 -1로 하면 버그가 생길 가능성이 있어서 0으로 수정
🤖 AI Code Review (Gemini)안녕하세요, NextRoom 프로젝트의 NR-122 Pull Request에 대한 코드 리뷰를 시작하겠습니다. 이번 PR은 타이머 화면과 힌트 화면이 💡 전반적인 피드백
🟡 WarningFile: fun setCurrentHint(hint: Hint) {
_currentHint.value = hint
}
fun updateCurrentHint(hint: Hint) {
_currentHint.value = hint
}
💡 SuggestionFile: private fun initSubscribe() {
viewLifecycleOwner.repeatOnStarted {
launch {
gameSharedViewModel.currentHint.collect { hint ->
hint?.let { viewModel.setHint(it) }
}
}
launch {
gameSharedViewModel.subscribeStatus.collect { subscribeStatus ->
viewModel.setSubscribeStatus(subscribeStatus)
}
}
}
}
하지만 예시 (고려해볼 수 있는 대안): // HintViewModel.kt
@HiltViewModel
class HintViewModel @Inject constructor(
private val timerRepository: TimerRepository,
private val dataStoreRepository: DataStoreRepository,
private val gameSharedViewModel: GameSharedViewModel, // GameSharedViewModel 주입
) : BaseViewModel<HintState, HintEvent>() {
override val container: Container<HintState, HintEvent> = container(HintState())
init {
// ViewModel 내부에서 GameSharedViewModel의 Flow를 구독
viewModelScope.launch {
gameSharedViewModel.currentHint.collect { hint ->
hint?.let { setHint(it) }
}
}
viewModelScope.launch {
gameSharedViewModel.subscribeStatus.collect { subscribeStatus ->
setSubscribeStatus(subscribeStatus)
}
}
// ... 기존 로직
}
// ... setHint, setSubscribeStatus 등 기존 함수
}
// HintFragment.kt
// initSubscribe() 제거
// viewModel은 그대로 사용장점:
단점:
현재 구현도 유효하며, 이 제안은 향후 유사한 패턴이 반복될 경우 고려해볼 만한 대안으로 제시합니다. 💡 SuggestionFile: data class Hint(
val id: Int = 0,
val progress: Int = 0,
val hint: String = "",
val answer: String = "",
val answerOpened: Boolean = false, // 추가
val hintImageUrlList: List<String> = emptyList(), // 추가
val answerImageUrlList: List<String> = emptyList(), // 추가
) : Serializable
// TimerViewModel.kt
// ...
TimerEvent.OnOpenHint(
hint = Hint(
id = hint.id,
progress = hint.progress,
hint = hint.hint,
answer = hint.answer,
answerOpened = state.answerOpenedHints.contains(hint.id),
hintImageUrlList = hint.hintImageUrlList.toList(),
answerImageUrlList = hint.answerImageUrlList.toList()
)
)
// ...
💡 SuggestionFile: class HintViewModel @Inject constructor(
private val timerRepository: TimerRepository,
// private val statsRepository: StatisticsRepository, // 제거
// private val adminRepository: AdminRepository, // 제거
private val dataStoreRepository: DataStoreRepository,
) : BaseViewModel<HintState, HintEvent>() {주석 처리된 💡 SuggestionFile: data class HintState(
val loading: Boolean = false,
val hint: Hint = Hint(), // 변경됨
val lastSeconds: Int = 0,
val userSubscribeStatus: SubscribeStatus = SubscribeStatus.Default,
val networkDisconnectedCount: Int = 0
)
Review Summary이번 PR은 제시된 Warning과 Suggestion들은 대부분 코드의 일관성과 명확성을 높이기 위한 제안이며, 현재 구현이 동작하는 데 큰 문제는 없습니다. 특히 전반적으로 잘 작성된 코드이며, 프로젝트의 Clean Architecture 및 Orbit MVI 패턴을 잘 준수하고 있습니다. 최종 승인 여부: 승인 (Approval) 위에서 언급된 Warning과 Suggestion들을 고려하여 개선하면 더욱 완벽한 코드가 될 것입니다. This review was automatically generated using Gemini AI. Please use your judgment when addressing the feedback. |
왜? setCurrentHint와 역할이 겹쳐 제거한다.
as-is) 타이머 화면에서 힌트 화면으로 넘어갈 때 argument를 넘겨준다.
to-be) 하나의 viewModel을 같이 공유해서 사용하도록 한다. (graph viewModel)
왜?
힌트 화면에서 진행률만 확인하고 힌트 차감없이 되돌아가는 기능이 새로 개발될 예정임
기존에는 힌트를 차감하고 다음 화면으로 넘어가는 방식이였으나
앞으로는 힌트 화면에서 데이터의 변경을 담당하게 될거임.
근데 이걸 setFragmentResult로 하면 다소 코드가 너저분해지고
나중에 기능이 확장될 것을 생각하면 이 방식은 누가 누구에게 무엇을 넘겼는지 파악하기 어려워짐