Navigation Base 로직 구현 #21
Conversation
📝 WalkthroughWalkthrough
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 리뷰 의견긍정적인 측면우수한 구조 설계 ✨
명확한 책임 분리
개선을 위한 제안1. 스타트 목적지 결정 로직의 견고성 🤔
val startDestination = contributors
.find { it.graphRoute == NavRoutes.LoginGraph }
?.startDestination
?: error("No LoginGraph contributor found")문제점: 런타임에 실패합니다. 개발 초기에는 의도가 명확하지만, 나중에 피처 모듈 누락 시 앱이 크래시됩니다. 개선 방안:
2. 우선순위 정렬의 일관성 📋
val contributors = rememberSaveable { /* 정렬 로직 */ }질문: 동일 우선순위( .sortedWith(compareBy({ it.priority }, { it.graphRoute.route }))이렇게 이차 정렬 기준을 추가하면 결정적(deterministic)인 동작을 보장할 수 있습니다. 3.
fun createRoute(id: String): String {
return "home_detail/$id"
}개선 제안:
4. 문서화 추가 제안 📚
/**
* 네비게이션 그래프를 동적으로 등록하는 기여자 인터페이스입니다.
* 각 피처 모듈은 이 인터페이스를 구현하여 자신의 네비게이션 그래프를 제공합니다.
*
* `@property` graphRoute 이 기여자가 정의하는 그래프의 라우트
* `@property` startDestination 그래프 내 시작 목적지
* `@property` priority 등록 우선순위 (낮은 값이 먼저 등록됨)
*/
interface NavGraphContributor { ... }최종 평가선택 사항이 아닌 핵심 기능들의 구조를 잘 설계했습니다! 위의 제안들은 장기적인 유지보수성과 안정성을 위한 선택적 개선 사항입니다. 현재 구현은 의도가 명확하고 이전 가능하며, 의존성 주입(Koin)과의 통합도 깔끔합니다. 🎉 Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
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 (3)
core/navigation/src/main/java/com/twix/navigation/AppNavHost.kt (1)
23-26: 시작 destination 선택 로직을 개선할 수 있습니다.현재
LoginGraph가 하드코딩되어 있어 유연성이 떨어집니다. PR 설명에서 "예시용 임시 값"이라고 명시하셨지만, 향후 다음과 같은 개선을 고려해보세요:개선 방안:
- 파라미터로 받기:
AppNavHost(startGraph: NavRoutes = NavRoutes.LoginGraph)형태로 외부에서 주입- 우선순위 기반: 가장 낮은 priority를 가진 contributor를 자동으로 시작점으로 사용
- 애노테이션 기반:
@StartGraph같은 애노테이션으로 표시에러 메시지도 더 구체적으로 만들 수 있습니다:
🔎 에러 메시지 개선 제안
- val start = contributors - .firstOrNull { it.graphRoute == NavRoutes.LoginGraph } - ?.graphRoute - ?: error("해당 Graph를 찾을 수 없습니다.") + val start = contributors + .firstOrNull { it.graphRoute == NavRoutes.LoginGraph } + ?.graphRoute + ?: error("LoginGraph를 찾을 수 없습니다. 등록된 그래프: ${contributors.map { it.graphRoute }}")core/navigation/src/main/java/com/twix/navigation/NavRoutes.kt (2)
3-10: 타입 안전한 Navigation 설계가 훌륭합니다!sealed class를 활용한 route 관리는 다음과 같은 장점이 있습니다:
✅ 현재 구조의 장점:
- 컴파일 타임에 route 오타 방지
- IDE 자동완성 지원
- when expression에서 exhaustive checking 가능
- 명확한 네이밍 컨벤션 정의
💡 선택적 개선 제안:
Kotlin 1.5+ 환경이라면
sealed interface를 고려해볼 수 있습니다:sealed interface NavRoutes { val route: String data object LoginGraph : NavRoutes { override val route = "login_graph" } // ... }sealed interface의 장점:
- 다중 상속 가능 (필요시 다른 인터페이스 구현)
- data object 사용으로 의미적으로 더 명확
- 향후 확장성 증가
하지만 현재 sealed class 구조도 충분히 좋으며, 이는 optional한 제안입니다.
17-19: 파라미터가 있는 route 처리가 잘 구현되었습니다!
createRoute()헬퍼 함수로 타입 안전하게 route를 생성하는 패턴이 훌륭합니다:✅ 장점:
- 수동 문자열 조합으로 인한 오류 방지
- IDE에서 사용법을 명확히 확인 가능
- 리팩토링 시 안전성 보장
💡 선택적 개선 아이디어:
향후 파라미터 검증이 필요한 경우 다음을 고려해볼 수 있습니다:
object HomeDetail: NavRoutes("home_detail/{id}") { fun createRoute(id: String): String { require(id.isNotBlank()) { "id는 비어있을 수 없습니다" } return "home_detail/$id" } }하지만 현재 구조도 충분히 실용적이며, validation은 필요시 추가하면 됩니다.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
core/navigation/build.gradle.ktscore/navigation/src/main/java/com/twix/navigation/AppNavHost.ktcore/navigation/src/main/java/com/twix/navigation/NavRoutes.ktcore/navigation/src/main/java/com/twix/navigation/base/NavGraphContributor.kt
🧰 Additional context used
📓 Path-based instructions (1)
**/*.gradle.kts
⚙️ CodeRabbit configuration file
**/*.gradle.kts: [Gradle 빌드 파일 리뷰 가이드]
- Convention Plugin이 적절히 사용되었는가?
- 불필요한 의존성이 없는가?
- implementation / api 구분이 명확한가?
- 모듈 간 의존성 방향이 올바른가?
Files:
core/navigation/build.gradle.kts
🧬 Code graph analysis (1)
core/navigation/src/main/java/com/twix/navigation/AppNavHost.kt (1)
core/navigation/src/main/java/com/twix/navigation/base/NavGraphContributor.kt (1)
registerGraph(16-16)
🔇 Additional comments (4)
core/navigation/build.gradle.kts (1)
4-4: Koin 플러그인 추가가 적절합니다.Convention Plugin을 통한 Koin 설정이 올바르게 적용되었습니다. AppNavHost에서
getKoin()과getAll<NavGraphContributor>()를 사용하기 위한 필수 설정이며, 모듈의 목적에 부합합니다.core/navigation/src/main/java/com/twix/navigation/AppNavHost.kt (2)
27-56: 애니메이션 설정이 잘 구현되었습니다!Navigation transition 설정이 일관성 있고 사용자 경험을 고려하여 구현되었습니다:
- duration 상수화: 매직 넘버를 상수로 추출하여 유지보수성 향상
- FastOutSlowInEasing: Material Design 권장 easing 함수 사용
- 양방향 애니메이션: enter/exit과 popEnter/popExit를 모두 구현하여 자연스러운 화면 전환 제공
애니메이션 방향도 논리적입니다:
- 새 화면 진입: 오른쪽 → 왼쪽 슬라이드
- 뒤로가기: 왼쪽 → 오른쪽 슬라이드
58-58: Graph 등록 로직이 깔끔합니다.
with(it) { registerGraph(navController) }패턴으로 각 contributor의 컨텍스트에서 extension function을 호출하는 방식이 명확합니다.NavGraphBuilder receiver 내에서 NavGraphContributor의
registerGraphextension function을 호출하는 구조가 타입 안전하게 구현되었습니다.core/navigation/src/main/java/com/twix/navigation/NavRoutes.kt (1)
11-15: 네이밍 컨벤션이 일관성 있게 적용되었습니다.Graph와 Screen의 네이밍 규칙이 명확하고 일관성 있게 적용되어 코드 가독성이 높습니다:
LoginGraph/HomeGraph: Graph는_graphsuffixLogin/Home: Screen은 소문자 형태이러한 컨벤션은 팀 내에서 route 추가 시 명확한 가이드라인을 제공합니다.
| val contributors = remember { | ||
| koin.getAll<NavGraphContributor>().sortedBy { it.priority } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# AppNavHost.kt 파일 찾기 및 내용 확인
find . -type f -name "AppNavHost.kt" | head -5Repository: YAPP-Github/Twix-Android
Length of output: 133
🏁 Script executed:
# NavGraphContributor 인터페이스 정의 찾기
rg -n --type=kotlin "interface NavGraphContributor|class NavGraphContributor"Repository: YAPP-Github/Twix-Android
Length of output: 180
🏁 Script executed:
# NavGraphContributor 구현 클래스 검색
rg -n --type=kotlin ":\s*NavGraphContributor" | head -20Repository: YAPP-Github/Twix-Android
Length of output: 50
🏁 Script executed:
cat -n ./core/navigation/src/main/java/com/twix/navigation/AppNavHost.kt | head -35Repository: YAPP-Github/Twix-Android
Length of output: 1595
🏁 Script executed:
cat -n ./core/navigation/src/main/java/com/twix/navigation/base/NavGraphContributor.ktRepository: YAPP-Github/Twix-Android
Length of output: 678
🏁 Script executed:
# 다른 패턴으로 NavGraphContributor 구현체 검색
rg -n --type=kotlin "NavGraphContributor" -A 1 -B 1 | grep -E "(class|object)" | head -20Repository: YAPP-Github/Twix-Android
Length of output: 50
🏁 Script executed:
# 모든 코틀린 파일에서 NavGraphContributor 참조 및 구현 찾기
rg -n --type=kotlin "NavGraphContributor" -B 2 -A 5 | grep -A 7 "^[0-9]*.*class\|^[0-9]*.*object"Repository: YAPP-Github/Twix-Android
Length of output: 50
🏁 Script executed:
# 다른 방식으로: object나 class 정의 후 : NavGraphContributor 찾기
ast-grep --pattern 'class $_ : $$$NavGraphContributor$$$'Repository: YAPP-Github/Twix-Android
Length of output: 50
🏁 Script executed:
# Koin 모듈 파일에서 NavGraphContributor 등록 확인
rg -n --type=kotlin "NavGraphContributor" -B 3 -A 3 | head -50Repository: YAPP-Github/Twix-Android
Length of output: 2335
🏁 Script executed:
# 프로젝트 구조 확인
ls -la | head -20Repository: YAPP-Github/Twix-Android
Length of output: 1224
🏁 Script executed:
# 모듈 목록 확인
find . -maxdepth 2 -type d -name "feature*" -o -name "*Graph*" 2>/dev/null | head -20Repository: YAPP-Github/Twix-Android
Length of output: 77
🏁 Script executed:
# 더 광범위한 NavGraphContributor 구현 검색
find . -type f -name "*.kt" -exec grep -l "NavGraphContributor" {} \; | head -30Repository: YAPP-Github/Twix-Android
Length of output: 213
🏁 Script executed:
# feature 모듈 내부 구조 확인
find ./feature -type f -name "*.kt" | head -20Repository: YAPP-Github/Twix-Android
Length of output: 129
🏁 Script executed:
# Koin 모듈 설정 파일 찾기
rg -n --type=kotlin "module\s*{|NavGraphContributor" -B 2 -A 5Repository: YAPP-Github/Twix-Android
Length of output: 190
🏁 Script executed:
# 프로젝트 전체에서 "koin.getAll" 또는 "single<NavGraphContributor>" 검색
rg -n --type=kotlin "(getAll|single).*NavGraphContributor|factory.*NavGraphContributor"Repository: YAPP-Github/Twix-Android
Length of output: 202
🏁 Script executed:
# Koin 관련 설정 파일 모두 찾기
find . -type f -name "*Koin*.kt" -o -name "*Module*.kt" -o -name "*Di*.kt" 2>/dev/null | head -20Repository: YAPP-Github/Twix-Android
Length of output: 152
🏁 Script executed:
# app 모듈의 구조 확인
find ./app -type f -name "*.kt" | head -20Repository: YAPP-Github/Twix-Android
Length of output: 120
🏁 Script executed:
# 모든 코틀린 파일에서 "module {" 패턴 찾기
rg -n --type=kotlin "^\s*module\s*\{" -A 10 | head -50Repository: YAPP-Github/Twix-Android
Length of output: 50
🏁 Script executed:
cat -n ./app/src/main/java/com/yapp/twix/TwixApplication.ktRepository: YAPP-Github/Twix-Android
Length of output: 526
🏁 Script executed:
cat -n ./build-logic/convention/src/main/kotlin/com/twix/convention/KoinConventionPlugin.kt | head -50Repository: YAPP-Github/Twix-Android
Length of output: 444
🏁 Script executed:
# 전체 코드베이스에서 NavGraphContributor를 구현한 클래스 또는 object 찾기
rg -n --type=kotlin "class\s+\w+.*:\s*.*NavGraphContributor|object\s+\w+.*:\s*.*NavGraphContributor"Repository: YAPP-Github/Twix-Android
Length of output: 50
🏁 Script executed:
# modules() 함수 정의 찾기
rg -n --type=kotlin "fun\s+modules\s*\(" -A 20Repository: YAPP-Github/Twix-Android
Length of output: 50
🏁 Script executed:
# 혹은 modules 변수 정의
rg -n --type=kotlin "val\s+modules.*=|object.*modules"Repository: YAPP-Github/Twix-Android
Length of output: 50
🏁 Script executed:
# TwixApplication.kt 파일의 import 문을 포함한 전체 내용
cat -n ./app/src/main/java/com/yapp/twix/TwixApplication.ktRepository: YAPP-Github/Twix-Android
Length of output: 526
🏁 Script executed:
# app 모듈 전체 파일 목록
find ./app -type f -name "*.kt" -o -name "*.xml"Repository: YAPP-Github/Twix-Android
Length of output: 570
🏁 Script executed:
# modules 함수 또는 변수 전역 검색 (확장함수 포함)
rg -n "modules" --type=kotlin | grep -E "(fun|val|object|package|import)" | head -30Repository: YAPP-Github/Twix-Android
Length of output: 50
🏁 Script executed:
# startKoin 호출 주변의 모든 코드 확인
rg -n --type=kotlin "startKoin" -B 5 -A 10Repository: YAPP-Github/Twix-Android
Length of output: 1269
🏁 Script executed:
# app 모듈의 build.gradle.kts 확인
find ./app -name "build.gradle.kts" -exec cat -n {} \;Repository: YAPP-Github/Twix-Android
Length of output: 1152
🏁 Script executed:
# 전체 코드베이스에서 "modules()" 함수 정의 찾기
rg -n --type=kotlin "fun.*modules\s*\(" -A 3Repository: YAPP-Github/Twix-Android
Length of output: 50
🏁 Script executed:
# Koin 설정 관련 파일 모두 찾기
find . -path ./build-logic -prune -o -type f -name "*.kt" -exec grep -l "module\s*{" {} \; 2>/dev/null | head -20Repository: YAPP-Github/Twix-Android
Length of output: 50
🏁 Script executed:
# twix.koin 플러그인 찾기
find ./build-logic -name "*koin*" -o -name "*Koin*" | grep -i kotlinRepository: YAPP-Github/Twix-Android
Length of output: 152
🏁 Script executed:
# feature/login 모듈 구조
find ./feature/login -type f -name "*.kt" | head -20Repository: YAPP-Github/Twix-Android
Length of output: 129
🏁 Script executed:
# feature/login 모듈에서 NavGraphContributor 관련 검색
rg -n --type=kotlin "NavGraphContributor" ./feature/login -A 5 -B 5Repository: YAPP-Github/Twix-Android
Length of output: 50
🏁 Script executed:
# 직접 전체 feature/login 디렉토리 구조 확인
find ./feature/login -type fRepository: YAPP-Github/Twix-Android
Length of output: 322
🏁 Script executed:
# LoginActivity.kt 내용 확인
cat -n ./feature/login/src/main/java/com/twix/login/LoginActivity.ktRepository: YAPP-Github/Twix-Android
Length of output: 382
🏁 Script executed:
# 전체 코드베이스에서 실제로 NavGraphContributor를 상속받는 구현 검색
rg -n "NavGraphContributor" --type=kotlin -cRepository: YAPP-Github/Twix-Android
Length of output: 213
NavGraphContributor 구현체가 없어 현재 코드가 실행되지 않는 상태입니다.
코드베이스 전체를 검색한 결과, NavGraphContributor 인터페이스는 정의되어 있으나 이를 구현한 클래스가 존재하지 않습니다. 따라서 koin.getAll<NavGraphContributor>()는 항상 빈 리스트를 반환하며, 결과적으로 23-26줄의 error("해당 Graph를 찾을 수 없습니다.")가 필연적으로 실행됩니다.
이를 해결하기 위해서는:
- feature 모듈들에서
NavGraphContributor구현체 작성 - 각 feature(예: login, home 등)마다 해당 그래프를 등록하는 구현체 필요 - Koin 모듈 설정 완성 - TwixApplication에서 호출하는
modules()함수 정의 및 각 feature의 Koin 모듈 연결 - 에러 메시지 개선 (필요시) - 현재 에러 메시지도 명확하지만, 구현체 등록 후 다음과 같이 개선 가능:
val start = contributors .firstOrNull { it.graphRoute == NavRoutes.LoginGraph } ?.graphRoute ?: error("LoginGraph를 찾을 수 없습니다. 등록된 그래프: ${contributors.map { it.graphRoute }}")
🤖 Prompt for AI Agents
core/navigation/src/main/java/com/twix/navigation/AppNavHost.kt lines 20-22:
koin.getAll<NavGraphContributor>()가 빈 리스트를 반환하여 error(...)가 실행되는 문제를 고치려면 각
feature 모듈(login, home 등)에 NavGraphContributor 인터페이스 구현체를 추가하고 해당 구현체들을 포함한 Koin
모듈을 작성하여 TwixApplication의 modules() 호출에 등록하세요; 또한 Koin 설정이 누락됐는지 확인하고 통합 테스트로
koin.getAll이 기대하는 구현체들을 반환하는지 검증하며(선택 사항) 런타임에 바로 크래시 대신 등록된 그래프 목록을 출력하도록 error
메시지를 더 구체화해 주세요.
There was a problem hiding this comment.
@dogmania, 네, feature 모듈에서 구현 예정이시군요! 이해했습니다.
추후 디버깅을 쉽게 하기 위해 다음과 같은 개선을 고려해보시면 좋을 것 같습니다 (선택 사항):
val contributors = remember {
koin.getAll<NavGraphContributor>().sortedBy { it.priority }
}
// TODO: feature 모듈에서 NavGraphContributor 구현체 추가 필요
val start = contributors
.firstOrNull { it.graphRoute == NavRoutes.LoginGraph }
?.graphRoute
?: error(
"LoginGraph를 찾을 수 없습니다. " +
"등록된 그래프 수: ${contributors.size}, " +
"그래프 목록: ${contributors.map { it.graphRoute }}"
)이렇게 하면:
- TODO 주석으로 구현이 필요한 부분을 명시
- 에러 발생 시 등록된 그래프 정보를 출력하여 누락된 구현체를 쉽게 파악 가능
하지만 현재 구조도 충분히 명확하므로, 선호하시는 방식으로 진행하셔도 됩니다! 👍
| interface NavGraphContributor { | ||
| val graphRoute: NavRoutes | ||
| val startDestination: String | ||
| val priority: Int get() = 100 | ||
|
|
||
| fun NavGraphBuilder.registerGraph(navController: NavHostController) | ||
| } No newline at end of file |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n --type=kotlin -C 3 'startDestination'Repository: YAPP-Github/Twix-Android
Length of output: 1528
🏁 Script executed:
# AppNavHost.kt 상세 확인
cat -n core/navigation/src/main/java/com/twix/navigation/AppNavHost.kt | sed -n '15,45p'Repository: YAPP-Github/Twix-Android
Length of output: 1281
🏁 Script executed:
# NavGraphContributor의 구현체 찾기
rg -n --type=kotlin 'class.*:.*NavGraphContributor'Repository: YAPP-Github/Twix-Android
Length of output: 50
🏁 Script executed:
# startDestination 프로퍼티 직접 사용처 검색 (contributor.startDestination 패턴)
rg -n --type=kotlin '\.startDestination'Repository: YAPP-Github/Twix-Android
Length of output: 50
🏁 Script executed:
# NavGraphContributor를 구현하는 모든 파일 찾기
rg -n --type=kotlin 'NavGraphContributor' -A 10 | head -50Repository: YAPP-Github/Twix-Android
Length of output: 2681
🏁 Script executed:
# startDestination에 대한 모든 언급 찾기 (주석 포함)
rg -n --type=kotlin --type-add='kt:include:*.kt' 'startDestination' -B 2 -A 2Repository: YAPP-Github/Twix-Android
Length of output: 127
🏁 Script executed:
# 간단하게 startDestination 모든 언급 찾기
grep -rn "startDestination" --include="*.kt" core/Repository: YAPP-Github/Twix-Android
Length of output: 288
startDestination 프로퍼티는 현재 사용되지 않고 있습니다. 제거하거나 명확히 문서화해야 합니다.
NavGraphContributor 인터페이스의 전반적인 설계는 우수합니다:
✅ 장점:
NavRoutes타입으로 타입 안전성 확보priority기본값 제공으로 대부분의 경우 간편하게 사용 가능- extension function 패턴으로 Compose Navigation과 자연스럽게 통합
startDestination 프로퍼티가 정의되어 있지만 코드 전체에서 실제로 사용되지 않습니다. 현재 AppNavHost에서는 graphRoute만 접근하여 시작 지점을 결정하고 있습니다.
개선 방안:
- 단기: 현재 사용되지 않는 프로퍼티이므로 제거
- 또는 중첩된 navigation graph의 시작점을 지정하기 위한 의도된 설계라면,
registerGraph구현에서 실제로 활용하고 주석으로 명확히 문서화
🤖 Prompt for AI Agents
In core/navigation/src/main/java/com/twix/navigation/base/NavGraphContributor.kt
around lines 11 to 17, the startDestination property is declared but never used;
either remove the unused property from the interface to avoid dead API surface,
or, if it was intended to mark nested graph entry points, update the interface
docs and the registerGraph implementations to consume startDestination (e.g.,
use it when building a nested NavGraph) and add a KDoc comment on
startDestination explaining its purpose and expected usage; choose one approach
and apply it consistently across all implementers and callers.
There was a problem hiding this comment.
:feature:*에서 NavGraphContributor 구현체를 만들 때 사용할 예정
There was a problem hiding this comment.
@dogmania, 이해했습니다! 현재는 기본 인프라를 구축하는 단계이고, 실제 feature 모듈에서 NavGraphContributor를 구현할 때 startDestination이 활용될 예정이군요.
이러한 설계는 합리적입니다. 각 feature의 navigation graph는 여러 화면으로 구성될 수 있고, startDestination으로 해당 그래프 내의 시작 화면을 명시하는 것이 필요합니다.
다만 향후 유지보수를 위해 KDoc에 사용 예시를 추가하는 것을 고려해보세요:
/**
* Navigation Graph 등록을 위한 기본 인터페이스
*
* priority는 우선적으로 등록해야 하는 Graph에서 값을 낮춰 사용하면 됩니다.
* ex) SplashGraph는 앱 내에서 가장 먼저 실행되어야 하는 그래프이므로 priority를 낮춰주면 됩니다.
*
* @property graphRoute 전체 앱에서 해당 그래프를 식별하는 route
* @property startDestination 해당 그래프 내에서 시작 화면의 route
* @property priority 그래프 등록 우선순위 (낮을수록 먼저 등록)
*/
chanho0908
left a comment
chanho0908
left a comment
There was a problem hiding this comment.
고생했어 !! 실제로 써봐야 좀 와닿을 것 같군 👍
이슈 번호
작업내용
리뷰어에게 추가로 요구하는 사항 (선택)