NR-146 비밀번호 확인 및 설정화면을 compose로 마이그레이션

[juhwankim-dev]

비밀번호 확인 및 설정 화면을 compose로 마이그레이션한다 !

순차적 마이그레이션 중 😉

특이사항 없습니당

[github-actions]

🤖 AI Code Review (Gemini)

안녕하세요! 비밀번호 확인 및 설정 화면을 Compose로 마이그레이션하는 PR에 대한 코드 리뷰를 시작하겠습니다. 전반적으로 Compose 마이그레이션이 프로젝트의 Clean Architecture 및 최신 Android 개발 가이드라인을 잘 따르고 있으며, 기존 View Binding 기반 코드에서 Compose 기반 코드로의 전환이 깔끔하게 이루어졌습니다. 특히 ViewModel의 상태 관리 방식 개선과 Compose UI 컴포넌트의 재사용성 고려가 돋보입니다.

---

💡 전반적인 피드백

• 긍정적인 측면:
  • Compose로의 성공적인 마이그레이션: 기존 XML 레이아웃과 View Binding 코드를 제거하고 Compose로 UI를 재구현한 점이 좋습니다.
  • ViewModel 개선: NewBaseViewModel을 사용하여 Orbit MVI를 벗어나고, UiState를 통해 상태를 명확하게 관리하는 방식으로 전환한 점은 프로젝트의 마이그레이션 전략에 부합하며 코드 가독성과 유지보수성을 높였습니다.
  • 재사용 가능한 Compose 컴포넌트: PinKeypad와 같은 컴포넌트를 별도로 분리하여 재사용성을 높인 점이 훌륭합니다.
  • 에러 처리 및 UX 개선: 비밀번호 불일치 시 에러 상태를 시각적으로 표시하고 일정 시간 후 초기화하는 로직이 추가되어 사용자 경험이 개선되었습니다.
  • ComposeView의 setViewCompositionStrategy(DisposeOnViewTreeLifecycleDestroyed) 사용은 Compose 뷰의 생명주기 관리를 올바르게 처리하는 좋은 예시입니다.

---

🟡 Warning

1. presentation/src/main/java/com/nextroom/nextroom/presentation/ui/password/CheckPasswordFragment.kt

• Line 60: private val showBiometric: Boolean = Build.VERSION.SDK_INT >= Build.VERSION_CODES.P
• Line 77: if (Build.VERSION.SDK_INT < Build.VERSION_CODES.P) return
  • 내용: showBiometric 프로퍼티를 통해 Compose UI에서 생체 인식 버튼의 표시 여부를 이미 제어하고 있습니다. showBiometricPrompt() 함수 내부의 SDK 버전 체크는 중복될 수 있습니다. 현재 코드에서는 onViewCreated에서 한 번 호출되고, Compose UI에서 버튼 클릭 시 호출되므로, Compose UI에서 버튼이 표시되지 않으면 클릭될 일이 없기 때문에 내부 체크는 불필요할 수 있습니다. 하지만 onViewCreated에서 호출될 때는 여전히 유효한 방어 로직이므로, 제거하지 않아도 큰 문제는 없습니다. 다만, 중복을 줄이고 싶다면 onViewCreated에서 if (showBiometric) showBiometricPrompt()와 같이 호출하여 명확하게 할 수 있습니다.
  • 제안: 현재 상태로 유지해도 무방하나, onViewCreated에서 showBiometricPrompt()를 호출하기 전에 if (showBiometric) 조건을 추가하여 명확성을 높일 수 있습니다.

2. presentation/src/main/java/com/nextroom/nextroom/presentation/ui/password/CheckPasswordViewModel.kt

• Line 20: private val _uiEvent = MutableSharedFlow<UiEvent>(extraBufferCapacity = 1)
• Line 21: val uiEvent = _uiEvent.asSharedFlow()
• Line 43: _uiEvent.emit(UiEvent.PasswordCorrect)
• Line 50: _uiEvent.emit(UiEvent.PasswordInCorrect)
  • 내용: MutableSharedFlow에 extraBufferCapacity = 1을 설정한 것은 단발성 이벤트를 처리할 때 유용합니다. 하지만 _uiEvent.emit() 호출 시 baseViewModelScope.launch 블록 내에서 emit을 호출하고 있으므로, emit은 suspend 함수입니다. 이 경우 extraBufferCapacity가 없어도 정상적으로 동작할 가능성이 높습니다. extraBufferCapacity는 주로 tryEmit을 사용하여 버퍼에 즉시 이벤트를 추가할 때 유용합니다. 현재 코드에서는 emit을 사용하고 있으므로, extraBufferCapacity의 효과가 제한적일 수 있습니다.
  • 제안: emit 대신 tryEmit을 사용하거나, MutableSharedFlow의 기본 동작을 신뢰하고 extraBufferCapacity를 제거하는 것을 고려해볼 수 있습니다. 다만, 현재 방식도 동작에는 문제가 없을 것입니다.

3. presentation/src/main/java/com/nextroom/nextroom/presentation/ui/password/SetPasswordViewModel.kt

• Line 20: private val _uiEvent = MutableSharedFlow<UiEvent>(extraBufferCapacity = 1)
• Line 21: val uiEvent = _uiEvent.asSharedFlow()
• Line 65: _uiEvent.emit(UiEvent.SettingPasswordFinished)
• Line 74: _uiEvent.emit(UiEvent.PasswordNotMatched)
  • 내용: CheckPasswordViewModel과 동일한 내용입니다. MutableSharedFlow의 extraBufferCapacity 사용과 emit 호출 방식에 대한 고려가 필요합니다.
  • 제안: CheckPasswordViewModel과 동일합니다.

---

💡 Suggestion

1. presentation/src/main/java/com/nextroom/nextroom/presentation/ui/password/CheckPasswordViewModel.kt

• Line 30: fun onNumberClicked(number: Int)
• Line 39: fun onBackSpaceClicked()
  • 내용: onNumberClicked와 onBackSpaceClicked 함수는 baseViewModelScope.launch를 사용하지 않고 _uiState.update를 직접 호출하고 있습니다. 이는 UI 상태를 즉시 업데이트하는 데 적합합니다. 반면 checkPassword 함수는 baseViewModelScope.launch를 사용하여 비동기 작업을 수행합니다. 이러한 패턴은 명확하고 좋습니다.
  • 제안: 현재 방식이 명확하고 적절합니다.

2. presentation/src/main/java/com/nextroom/nextroom/presentation/ui/password/compose/PinKeypad.kt

• Line 13: private val KEY_HEIGHT = 64.dp
  • 내용: KEY_HEIGHT를 파일 상단에 정의한 것은 좋습니다.
  • 제안: PinKeypad 컴포저블 함수 내에서 사용되는 상수이므로, PinKeypad 컴패니언 오브젝트나 파일 스코프 대신 PinKeypad 함수 내부에서 val KEY_HEIGHT = 64.dp로 정의하거나, PinKeypad 컴포저블 함수 외부에 private const val KEY_HEIGHT_DP = 64와 같이 const로 정의하고 KEY_HEIGHT_DP.dp로 사용하는 것도 고려해볼 수 있습니다. 현재 방식도 큰 문제는 없습니다.

---

✅ 리뷰 요약

이 PR은 비밀번호 확인 및 설정 화면을 Jetpack Compose로 성공적으로 마이그레이션했습니다.

• 코드 품질: Kotlin 컨벤션, Android 베스트 프랙티스, Clean Architecture 원칙을 잘 따르고 있습니다. ViewModel의 상태 관리 방식이 개선되었고, Compose UI 컴포넌트의 재사용성을 고려한 설계가 돋보입니다.
• 잠재적 버그/이슈: 특별한 버그나 메모리 누수, 스레드 안전성 문제는 발견되지 않았습니다.
• 프로젝트 가이드라인 준수: Compose 마이그레이션 전략, ViewModel의 NewBaseViewModel 사용, Hilt DI 패턴 등이 프로젝트 가이드라인에 부합합니다.
• 성능 최적화: Compose의 효율적인 리컴포지션 메커니즘을 잘 활용하고 있으며, 불필요한 객체 할당이나 비효율적인 로직은 보이지 않습니다.

전반적으로 매우 잘 작성된 PR이며, 제안된 사항들은 개선을 위한 선택 사항일 뿐, 현재 코드에 심각한 문제는 없습니다.

---

terminal-notifier -title "Claude Code" -message "비밀번호 화면 Compose 마이그레이션 PR 코드 리뷰 완료" -sound default

---

This review was automatically generated using Gemini AI. Please use your judgment when addressing the feedback.

[juhwankim-dev]

리뷰 항목중 위험도가 높거나 수정이 필요해 보이는 것이 없어 이번 PR은 반영 없이 머지합니다.