refactor: 회원가입 로직 수정 #691
refactor: 회원가입 로직 수정 #691
Conversation
Walkthrough이 변경사항은 네비게이터 관련 메서드의 시그니처를 Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Activity/ViewModel/Composable
participant Navigator as WebsosoNavigator
participant System as Android System
Caller->>Navigator: navigateToLoginActivity(::startActivity)
Navigator->>Caller: (생성된 Intent 전달)
Caller->>System: startActivity(Intent)
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainI'll help you review this pull request, @s9hn. Let me gather the necessary information about PR #691 first. 🏁 Script executed: Length of output: 1393 Let me get the details of the changes in this PR to provide a thorough review. 🏁 Script executed: Length of output: 17643 @s9hn I've reviewed PR #691 which refactors the sign-up logic and navigation pattern. This is a very solid refactoring that improves the codebase in several important ways: Navigation Pattern Improvements
Authentication Logic Improvements
These changes make the codebase more maintainable and follow good software engineering practices by improving separation of concerns. The PR significantly reduces code (91 deletions vs 24 additions) while making it more modular. Overall, this is a high-quality refactoring that improves the architecture. Approved! 👍 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/src/main/java/com/into/websoso/core/common/util/navigator/WebsosoNavigator.kt (1)
1-45: 함수형 인터페이스를 활용한 네비게이터 리팩토링에 대한 종합 의견전체적인 네비게이터 구현이 함수형 프로그래밍 패러다임을 더 잘 활용하도록 개선되었습니다. 이러한 접근 방식은 다음과 같은 이점을 제공합니다:
- 의존성 주입 개선: 컨텍스트를 직접 사용하는 대신 함수 참조를 통해 액티비티 시작 로직을 주입
- 테스트 용이성: 실제 네비게이션 없이도 함수 호출 여부를 검증 가능
- 모듈 간 결합도 감소: 특정 컨텍스트나 라이프사이클에 덜 의존적
- 코드 재사용성 향상: 다양한 환경에서 동일한 네비게이션 로직 사용 가능
다만, 이 패턴을 적용하면서 모든 호출 지점이 올바르게 업데이트되었는지 확인하는 것이 중요합니다. PR 설명에 따르면 모든 관련 파일이 수정되었으므로 문제가 없을 것으로 보입니다.
app/src/main/java/com/into/websoso/data/repository/AuthRepository.kt (1)
15-15: 표준화된 Result 래퍼 반환 검토 권장
fetchNicknameValidity가Boolean만 반환하면서 예외는 그대로 전파하도록 설계되었습니다. 호출부에서는 매번runCatching { … }또는try‐catch로 감싸야 하므로 중복 코드가 늘어납니다.
Result<Boolean>또는 자체sealed class(예:Resource.Success,Resource.Error)를 반환하도록 리포지터리 레이어에서 통합 처리하면,
- 호출부 간결화
- 오류 유형 중앙 관리
- 테스트 용이성
을 기대할 수 있습니다.
필요 시 도입을 검토해 주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
app/src/main/java/com/into/websoso/WebsosoApp.kt(1 hunks)app/src/main/java/com/into/websoso/core/common/util/navigator/WebsosoNavigator.kt(2 hunks)app/src/main/java/com/into/websoso/data/di/RepositoryModule.kt(0 hunks)app/src/main/java/com/into/websoso/data/remote/api/AuthApi.kt(0 hunks)app/src/main/java/com/into/websoso/data/repository/AuthRepository.kt(1 hunks)app/src/main/java/com/into/websoso/ui/accountInfo/LogoutDialogFragment.kt(1 hunks)app/src/main/java/com/into/websoso/ui/onboarding/OnboardingActivity.kt(0 hunks)app/src/main/java/com/into/websoso/ui/onboarding/OnboardingViewModel.kt(1 hunks)app/src/main/java/com/into/websoso/ui/splash/SplashActivity.kt(1 hunks)app/src/main/java/com/into/websoso/ui/withdraw/second/WithdrawSecondActivity.kt(1 hunks)core/common/src/main/java/com/into/websoso/core/common/navigator/NavigatorProvider.kt(1 hunks)feature/signin/src/main/java/com/into/websoso/feature/signin/SignInScreen.kt(3 hunks)
💤 Files with no reviewable changes (3)
- app/src/main/java/com/into/websoso/data/di/RepositoryModule.kt
- app/src/main/java/com/into/websoso/ui/onboarding/OnboardingActivity.kt
- app/src/main/java/com/into/websoso/data/remote/api/AuthApi.kt
🔇 Additional comments (13)
app/src/main/java/com/into/websoso/ui/splash/SplashActivity.kt (1)
42-43: 네비게이션 메서드 호출 방식의 일관성 있는 변경이제 NavigatorProvider의 메서드들이 Activity 컨텍스트에서
startActivity함수 참조를 받도록 변경되었습니다. 이 변경은 네비게이터 컴포넌트가 applicationContext 대신 앱 컨텍스트를 사용하도록 하는 PR 목표와 일치합니다.app/src/main/java/com/into/websoso/ui/accountInfo/LogoutDialogFragment.kt (1)
36-36: 네비게이션 호출 패턴 통일다른 Activity와 마찬가지로
navigateToLoginActivity메서드에::startActivity함수 참조를 전달하도록 수정되었습니다. 이 방식은 네비게이터가 Activity를 시작하는 방법에 대한 일관된 패턴을 제공합니다.app/src/main/java/com/into/websoso/WebsosoApp.kt (1)
32-32: Application 컨텍스트에서 네비게이션 메서드 호출 방식 개선Application 클래스에서도 네비게이션 메서드 호출 시
::startActivity함수 참조를 전달하도록 변경되었습니다. 이것은 PR에서 언급된 "네비게이터 컴포넌트가 applicationContext 대신 앱 컨텍스트를 사용하도록 수정" 목표와 일치합니다.app/src/main/java/com/into/websoso/ui/withdraw/second/WithdrawSecondActivity.kt (1)
107-107: 토큰 저장 로직 분리에 따른 네비게이션 메서드 호출 수정탈퇴 성공 후 로그인 화면으로 이동하는 코드가 새로운 패턴에 맞게 수정되었습니다. 이전에는
websosoNavigator.navigateToLoginActivity()를 호출했지만, 이제는 함수 참조::startActivity를 명시적으로 전달합니다. 이는 PR 목표인 "토큰 저장 로직을 모듈화하고 분리"하는 작업의 일환으로 보입니다.feature/signin/src/main/java/com/into/websoso/feature/signin/SignInScreen.kt (2)
15-15: LocalContext 추가로 컨텍스트 관리 개선LocalContext를 사용하여 현재 컴포즈 환경의 컨텍스트를 가져오는 방식은 적절합니다. 이렇게 하면 컴포저블 내에서 필요한 안드로이드 컨텍스트를 직접 접근할 수 있어 더 유연한 구현이 가능합니다.
Also applies to: 40-40
56-58: 네비게이터 호출 방식 개선컨텍스트의 startActivity 메서드를 함수 참조로 전달하여 네비게이션 책임을 호출자에게 위임하는 방식으로 변경한 것은 좋은 리팩토링입니다. 이 패턴은 다음과 같은 이점이 있습니다:
- 관심사 분리: 네비게이터는 Intent 생성에만 집중하고, 실행은 호출자가 담당
- 테스트 용이성: 실제로 액티비티를 시작하지 않고도 네비게이션 로직을 검증 가능
- 유연성 향상: 호출자가 필요에 따라 Intent에 추가 정보를 더하거나 다양한 방식으로 처리 가능
core/common/src/main/java/com/into/websoso/core/common/navigator/NavigatorProvider.kt (2)
3-3: Intent 임포트 추가Intent 클래스 임포트는 메서드 시그니처 변경에 따른 적절한 수정입니다.
9-13: 네비게이터 인터페이스 시그니처 개선네비게이션 메서드의 시그니처를
(Intent) -> Unit타입의 함수를 파라미터로 받도록 변경한 것은 좋은 설계 결정입니다. 이를 통해:
- 의존성 역전: 네비게이터가 액티비티 시작 책임을 직접 갖지 않고 호출자에게 위임
- 모듈 간 결합도 감소: 네비게이터 컴포넌트가 특정 컨텍스트에 덜 의존적
- 테스트 용이성: 함수형 인터페이스로 모킹이 더 쉬워짐
이러한 변경은 클린 아키텍처 원칙에 더 부합합니다.
app/src/main/java/com/into/websoso/core/common/util/navigator/WebsosoNavigator.kt (4)
4-4: Intent 임포트 추가Intent 클래스 임포트는 메서드 파라미터 변경에 따른 적절한 수정입니다.
22-25: navigateToLoginActivity 메서드 개선Intent 생성 후 호출자가 제공한 함수를 사용하여 액티비티를 시작하도록 변경한 것은 좋은 접근 방식입니다. 이를 통해 네비게이션 로직과 액티비티 시작 책임이 명확히 분리됩니다.
27-30: navigateToMainActivity 메서드 개선MainActivity Intent 생성 후 호출자의 함수로 실행을 위임하는 방식으로 변경되었습니다. 이전 코드와 비교했을 때 더 유연하고 테스트하기 쉬운 구조로 개선되었습니다.
32-35: navigateToOnboardingActivity 메서드 개선OnboardingActivity로의 네비게이션 역시 동일한 패턴으로 일관되게 개선되었습니다. 이제 모든 네비게이션 메서드가 동일한 패턴을 따르고 있어 코드의 일관성이 향상되었습니다.
app/src/main/java/com/into/websoso/data/repository/AuthRepository.kt (1)
9-10:@Singleton추가 👍
리포지터리가 상태를 가지지 않는 구조로 변경되었으므로 싱글톤 스코프로 제공하는 것은 적절한 선택입니다. Hilt의 기본 컴포넌트 범위와도 잘 맞습니다.
| viewModelScope.launch { | ||
| runCatching { | ||
| authRepository.fetchNicknameValidity(accessToken, nickname) | ||
| authRepository.fetchNicknameValidity(nickname) | ||
| }.onSuccess { isNicknameValid -> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
네트워크 호출은 IO 디스패처에서 수행하도록 분리하세요
viewModelScope.launch의 기본 디스패처는 Dispatchers.Main이기 때문에 현재 코드에서는 메인 스레드에서 API 호출이 실행될 수 있습니다. 레트로핏이 자체적으로 스레드 전환을 하도록 설정돼 있지 않다면 ANR 및 UI 지연이 발생할 여지가 있습니다.
아래와 같이 withContext(Dispatchers.IO)로 IO 스레드에서 실행되게 하시는 것을 권장드립니다.
viewModelScope.launch {
- runCatching {
- authRepository.fetchNicknameValidity(nickname)
- }.onSuccess { isNicknameValid ->
+ runCatching {
+ withContext(Dispatchers.IO) {
+ authRepository.fetchNicknameValidity(nickname)
+ }
+ }.onSuccess { isNicknameValid ->추가로, withContext 사용 시 다음 import 가 필요합니다.
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.withContext🤖 Prompt for AI Agents
In app/src/main/java/com/into/websoso/ui/onboarding/OnboardingViewModel.kt
around lines 98 to 101, the network call inside viewModelScope.launch is running
on the main thread by default, which can cause UI delays or ANRs. To fix this,
wrap the network call authRepository.fetchNicknameValidity(nickname) inside
withContext(Dispatchers.IO) to ensure it runs on the IO dispatcher. Also, add
the necessary imports for Dispatchers and withContext.
📌𝘐𝘴𝘴𝘶𝘦𝘴
📎𝘞𝘰𝘳𝘬 𝘋𝘦𝘴𝘤𝘳𝘪𝘱𝘵𝘪𝘰𝘯
💬𝘛𝘰 𝘙𝘦𝘷𝘪𝘦𝘸𝘦𝘳𝘴
Summary by CodeRabbit
Refactor
Bug Fixes