[ADD/#1] 프로젝트 기초 세팅 #2
Conversation
auto-assign
bot
requested review from
ShinHyeongcheol,
oilbeaneda and
vahkjsdf
seungjunGong
assigned
seungjunGong and unassigned
ShinHyeongcheol,
vahkjsdf and
oilbeaneda
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 안내Walkthrough프로젝트의 초기 설정을 구성합니다. 클린 아키텍처 기반의 계층 구조를 수립하고, Hilt 의존성 주입, Retrofit 네트워킹, DataStore 기반 토큰 관리, Jetpack Compose UI, 그리고 타입-안전 네비게이션을 통합합니다. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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: 9
🧹 Nitpick comments (21)
app/src/main/java/com/smashing/app/core/extension/StringExt.kt (2)
13-13: 함수 이름을 더 명확하게 개선하는 것을 고려하세요.
checkLength()는 검증(validation)을 수행하는 것처럼 보이지만, 실제로는 문자 개수를 반환합니다. 더 명확한 이름을 고려해보세요:
countGraphemes(): 그래핌 클러스터 개수를 센다는 의미graphemeLength(): 그래핌 기반 길이를 반환perceptualLength(): 사용자가 인식하는 길이이렇게 하면 API의 의도가 더 명확해집니다.
13-24:checkLength()함수는 현재 코드베이스에서 사용되지 않고 있습니다.구현 로직은 정확합니다.
BreakIterator.getCharacterInstance()를 사용한 그래핌 클러스터 카운팅이 올바르게 구현되어 있으며, 이모지와 결합 문자를 정확히 처리합니다.다만 검색 결과 이 유틸리티 함수는 현재 코드베이스의 어디에서도 호출되지 않고 있습니다. 미사용 코드는 제거하거나 실제 사용 사례를 추가해야 합니다.
app/src/main/java/com/smashing/app/data/local/datasource/impl/TokenDatasourceImpl.kt (1)
45-48: Companion 객체 이름 중복 제거 권장
companion object Companion에서Companion은 기본 이름이므로 명시할 필요가 없습니다.companion object만 사용하면 더 간결합니다.🔎 제안하는 수정
- companion object Companion { + companion object { private val ACCESS_TOKEN = stringPreferencesKey("ACCESS_TOKEN") private val REFRESH_TOKEN = stringPreferencesKey("REFRESH_TOKEN") }app/src/main/java/com/smashing/app/core/extension/ModifierExt.kt (1)
16-22: 리플 제거 로직이 올바르게 구현되었습니다.
indication = null과MutableInteractionSource를 사용하여 리플 효과를 제거하는 구현이 정확합니다. KDoc도 명확하게 작성되었습니다.
composed를 사용하면 매 리컴포지션마다 새로운 Modifier가 할당되는데, 다음과 같이 최적화를 고려할 수 있습니다:🔎 성능 최적화를 위한 대안
-fun Modifier.noRippleClickable(onClick: () -> Unit): Modifier = composed { - clickable( +fun Modifier.noRippleClickable(onClick: () -> Unit): Modifier = composed( + inspectorInfo = debugInspectorInfo { + name = "noRippleClickable" + properties["onClick"] = onClick + } +) { + this.clickable( indication = null, interactionSource = remember { MutableInteractionSource() }, onClick = onClick, ) }또는 싱글톤
MutableInteractionSource를 모듈 레벨에서 공유하여composed없이 사용할 수도 있지만, 이는 동시 인터랙션 추적에 영향을 줄 수 있어 현재 구현이 더 안전합니다.app/src/main/java/com/smashing/app/core/network/RetrofitExt.kt (1)
3-5: JSON 형식 감지 로직이 구현되었습니다.네트워크 로깅을 위한 JSON 형식 감지 함수가 추가되었습니다. 현재 구현은 시작/종료 문자만 확인하므로 실제 JSON 구조를 검증하지는 않습니다.
로깅 목적으로는 충분하지만, 향후 더 정확한 감지가 필요한 경우 다음을 고려할 수 있습니다:
🔎 개선 제안
fun String?.isJsonObject(): Boolean { if (this == null) return false val trimmed = this.trim() return trimmed.startsWith("{") && trimmed.endsWith("}") && trimmed.length >= 2 } fun String?.isJsonArray(): Boolean { if (this == null) return false val trimmed = this.trim() return trimmed.startsWith("[") && trimmed.endsWith("]") && trimmed.length >= 2 }또는 실제 JSON 파싱을 시도하여 유효성을 확인할 수도 있지만, 로깅 성능에 영향을 줄 수 있어 현재 구현이 실용적입니다.
app/src/main/res/values/strings.xml (1)
4-4: TODO 주석이 있습니다.
dummy문자열 리소스에 "삭제 예정" TODO 주석이 있습니다. 이 항목을 추적하고 적절한 시점에 제거하는 것을 잊지 마세요.이슈를 생성하여 추적하거나 제거 작업을 도와드릴까요?
app/src/main/java/com/smashing/app/core/extension/ContextExt.kt (1)
13-17: URL 검증 및 에러 로깅 추가를 권장합니다.현재 구현은 다음 문제가 있습니다:
- URL 검증 없이 Intent를 생성하므로, 사용자 입력이 전달될 경우 보안 문제가 될 수 있습니다
- 실패 시 로깅이 없어 디버깅이 어렵습니다
🔎 URL 검증 및 로깅 추가 제안
+import android.util.Log + /** * 주어진 URL을 기본 브라우저로 엽니다. * * @param url URL 문자열 * @return 성공 시 true, 실패 시 false */ fun Context.openUrl(url: String): Boolean { return runCatching { + val uri = url.toUri() + if (uri.scheme !in listOf("http", "https")) { + Log.w("ContextExt", "Invalid URL scheme: ${uri.scheme}") + return false + } - startActivity(Intent(Intent.ACTION_VIEW, url.toUri())) + startActivity(Intent(Intent.ACTION_VIEW, uri)) + }.onFailure { e -> + Log.e("ContextExt", "Failed to open URL: $url", e) }.isSuccess }app/src/main/java/com/smashing/app/presentation/home/component/DummyItem.kt (2)
44-53: 테마 색상 사용을 권장합니다.
Color.Gray를 직접 사용하면 다크 모드나 커스텀 테마에서 일관성이 떨어질 수 있습니다.🔎 MaterialTheme 색상 사용 제안
+import androidx.compose.material3.MaterialTheme + Text( text = email, fontSize = 10.sp, - color = Color.Gray, + color = MaterialTheme.colorScheme.onSurfaceVariant, )
31-36: 접근성 개선을 고려해주세요.프로필 이미지에
contentDescription을 추가하면 스크린 리더 사용자에게 도움이 됩니다.🔎 contentDescription 추가 제안
UrlImage( url = profileImage, + contentDescription = "$firstName $lastName의 프로필 이미지", modifier = Modifier .weight(1f) .aspectRatio(1f), )app/src/main/java/com/smashing/app/presentation/main/component/MainBottomBar.kt (1)
73-96:isSelected파라미터 활용 필요
isSelected파라미터가 전달되지만 시각적 구분에 활용되지 않고 있습니다. TODO 주석에 따라 디자인 적용 시 선택 상태에 따른 아이콘/텍스트 색상 변경이 필요합니다.app/src/main/java/com/smashing/app/core/designsystem/component/UrlImage.kt (2)
42-48: 로딩 및 에러 상태 처리 추가 권장
AsyncImage에placeholder,error,fallback파라미터가 설정되지 않아 이미지 로딩 중이거나 실패 시 빈 공간으로 표시됩니다. 사용자 경험 개선을 위해 상태별 이미지 표시를 권장합니다.🔎 수정 제안
} else { AsyncImage( model = url, contentDescription = contentDescription, contentScale = contentScale, modifier = modifier, + placeholder = painterResource(ic_fake_red), + error = painterResource(ic_fake_red), ) }
painterResourceimport 추가 필요:import androidx.compose.ui.res.painterResource
51-58: Preview 함수 접근 제한자 추가 권장Preview 함수는 외부에서 사용되지 않으므로
private으로 선언하는 것이 좋습니다.🔎 수정 제안
@Preview @Composable -fun UrlImagePreview() { +private fun UrlImagePreview() { UrlImage(app/src/main/java/com/smashing/app/presentation/main/MainTab.kt (1)
46-48:contains함수 간소화 가능
map후any를 호출하는 대신 직접any를 사용하면 중간 컬렉션 생성을 피할 수 있습니다.🔎 수정 제안
fun contains(predicate: (Route) -> Boolean): Boolean { - return entries.map { it.route }.any { predicate(it) } + return entries.any { predicate(it.route) } }app/src/main/java/com/smashing/app/presentation/main/MainAppState.kt (1)
80-85: DUMMY1, DUMMY2 탭 네비게이션 미구현
DUMMY1과DUMMY2탭 클릭 시 아무 동작도 하지 않습니다. 사용자가 해당 탭을 클릭해도 반응이 없어 혼란을 줄 수 있습니다. TODO 주석에 따라 추후 구현 예정이라면 해당 탭을 임시로 숨기거나, 최소한 로그를 남기는 것을 권장합니다.네비게이션 함수 구현이나 해당 탭 비활성화 처리를 도와드릴까요?
app/src/main/java/com/smashing/app/core/network/di/NetworkModule.kt (2)
32-36: 프로덕션에서prettyPrint비활성화 권장
prettyPrint = true는 JSON 직렬화 시 추가 오버헤드를 발생시킵니다. 디버그 빌드에서만 활성화하는 것을 권장합니다.🔎 수정 제안
@Provides @Singleton fun provideJson(): Json = Json { encodeDefaults = true ignoreUnknownKeys = true - prettyPrint = true + prettyPrint = BuildConfig.DEBUG }
65-71: OkHttpClient 타임아웃 설정 누락타임아웃 설정이 없으면 네트워크 지연이나 장애 시 무한 대기가 발생할 수 있습니다.
connectTimeout,readTimeout,writeTimeout설정을 권장합니다.🔎 수정 제안
+import java.util.concurrent.TimeUnit + @Provides @Singleton fun provideOkHttpClient( loggingInterceptor: Interceptor, ): OkHttpClient = OkHttpClient.Builder() .addInterceptor(loggingInterceptor) + .connectTimeout(30, TimeUnit.SECONDS) + .readTimeout(30, TimeUnit.SECONDS) + .writeTimeout(30, TimeUnit.SECONDS) .build()gradle/libs.versions.toml (1)
36-36: 오타 수정: "Thrid party" → "Third party"🔎 수정 제안
-# Thrid party +# Third partyapp/src/main/java/com/smashing/app/data/mapper/DummyMapper.kt (1)
6-12: 매퍼 함수가 잘 구성되어 있습니다.null-safe 처리를
orEmpty()및 기본값으로 적절히 처리하고 있습니다.다만, 실제 프로덕션 코드로 전환 시
id가 null인 경우를 0으로 처리하는 것이 비즈니스 로직에 적합한지 검토가 필요할 수 있습니다. 현재 Dummy 구현 단계에서는 문제없습니다.app/src/main/java/com/smashing/app/presentation/main/MainScreen.kt (1)
51-58: 화면 전환 애니메이션 의도 확인모든 화면 전환 애니메이션이
None으로 설정되어 있습니다. 이것이 의도된 디자인인지 확인하시기 바랍니다. 향후 부드러운 사용자 경험을 위해 전환 애니메이션 추가를 고려할 수 있습니다.app/src/main/java/com/smashing/app/presentation/home/HomeScreen.kt (1)
44-63: 빈 상태 화면 구현 필요
UiState의 모든 분기를 올바르게 처리하고 있습니다. 다만Idle,Failure,Loading상태에 대한 UI 구현이 아직 되어있지 않습니다. 향후 사용자 경험 향상을 위해 이들 상태에 대한 화면 구현을 추가하시기 바랍니다.app/src/main/java/com/smashing/app/presentation/home/HomeViewModel.kt (1)
24-34: 빈 리스트 상태 처리 재검토 제안데이터 로딩 로직이 전반적으로 잘 구현되어 있습니다. 다만 Lines 26-30에서 빈 리스트를
UiState.Idle로 처리하는 것은 재고해볼 여지가 있습니다.현재 로직:
- 빈 리스트 →
Idle(데이터 로딩 전 상태와 동일)- 데이터 있음 →
Success이렇게 하면 "아직 로딩하지 않음"과 "로딩했지만 결과가 비어있음"을 구분할 수 없습니다.
Success(emptyList)로 처리하면 의미가 더 명확해집니다.🔎 제안 수정
fun fetchDummyUsers() = viewModelScope.launch { + updateDummyUiState(UiState.Loading) dummyRepository.fetchDummyUserList(page = 1).onSuccess { userList -> - if (userList.isNotEmpty()) { - updateDummyUiState(UiState.Success(userList.toImmutableList())) - } else { - updateDummyUiState(UiState.Idle) - } + updateDummyUiState(UiState.Success(userList.toImmutableList())) }.onFailure { updateDummyUiState(UiState.Failure(it.message ?: "Unknown Error")) } }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (57)
.github/auto_assign.ymlapp/build.gradle.ktsapp/src/main/AndroidManifest.xmlapp/src/main/java/com/smashing/app/MainActivity.ktapp/src/main/java/com/smashing/app/SmashingApplication.ktapp/src/main/java/com/smashing/app/core/common/constant/.gitkeepapp/src/main/java/com/smashing/app/core/common/navigation/MainTabRoute.ktapp/src/main/java/com/smashing/app/core/common/navigation/Route.ktapp/src/main/java/com/smashing/app/core/common/state/UiState.ktapp/src/main/java/com/smashing/app/core/common/type/.gitkeepapp/src/main/java/com/smashing/app/core/designsystem/component/UrlImage.ktapp/src/main/java/com/smashing/app/core/designsystem/theme/Color.ktapp/src/main/java/com/smashing/app/core/designsystem/theme/Theme.ktapp/src/main/java/com/smashing/app/core/designsystem/theme/Type.ktapp/src/main/java/com/smashing/app/core/extension/ContextExt.ktapp/src/main/java/com/smashing/app/core/extension/FlowExt.ktapp/src/main/java/com/smashing/app/core/extension/LazyListStateExt.ktapp/src/main/java/com/smashing/app/core/extension/ModifierExt.ktapp/src/main/java/com/smashing/app/core/extension/StringExt.ktapp/src/main/java/com/smashing/app/core/local/datastore/di/DataStoreModule.ktapp/src/main/java/com/smashing/app/core/local/datastore/di/DataStoreQualifier.ktapp/src/main/java/com/smashing/app/core/network/RetrofitExt.ktapp/src/main/java/com/smashing/app/core/network/di/NetworkModule.ktapp/src/main/java/com/smashing/app/core/util/SuspendRunCatching.ktapp/src/main/java/com/smashing/app/data/di/auth/AuthDataSourceModule.ktapp/src/main/java/com/smashing/app/data/di/dummy/DummyDataSourceModule.ktapp/src/main/java/com/smashing/app/data/di/dummy/DummyRepositoryModule.ktapp/src/main/java/com/smashing/app/data/di/dummy/DummyServiceModule.ktapp/src/main/java/com/smashing/app/data/local/datasource/api/TokenDatasource.ktapp/src/main/java/com/smashing/app/data/local/datasource/impl/TokenDatasourceImpl.ktapp/src/main/java/com/smashing/app/data/mapper/DummyMapper.ktapp/src/main/java/com/smashing/app/data/model/DummyUser.ktapp/src/main/java/com/smashing/app/data/remote/datasource/api/DummyDataSource.ktapp/src/main/java/com/smashing/app/data/remote/datasource/impl/DummyDataSourceImpl.ktapp/src/main/java/com/smashing/app/data/remote/dto/GetUserListResponse.ktapp/src/main/java/com/smashing/app/data/remote/service/DummyService.ktapp/src/main/java/com/smashing/app/data/repository/api/DummyRepository.ktapp/src/main/java/com/smashing/app/data/repository/impl/DummyRepositoryImpl.ktapp/src/main/java/com/smashing/app/domain/mapper/.gitkeepapp/src/main/java/com/smashing/app/domain/model/.gitkeepapp/src/main/java/com/smashing/app/domain/usecase/.gitkeepapp/src/main/java/com/smashing/app/presentation/dummy/HomeNavigation.ktapp/src/main/java/com/smashing/app/presentation/home/HomeContract.ktapp/src/main/java/com/smashing/app/presentation/home/HomeScreen.ktapp/src/main/java/com/smashing/app/presentation/home/HomeViewModel.ktapp/src/main/java/com/smashing/app/presentation/home/component/DummyItem.ktapp/src/main/java/com/smashing/app/presentation/home/navigation/HomeNavigation.ktapp/src/main/java/com/smashing/app/presentation/home/type/.gitkeepapp/src/main/java/com/smashing/app/presentation/main/MainActivity.ktapp/src/main/java/com/smashing/app/presentation/main/MainAppState.ktapp/src/main/java/com/smashing/app/presentation/main/MainScreen.ktapp/src/main/java/com/smashing/app/presentation/main/MainTab.ktapp/src/main/java/com/smashing/app/presentation/main/component/MainBottomBar.ktapp/src/main/res/drawable/ic_fake_red.xmlapp/src/main/res/values/strings.xmlbuild.gradle.ktsgradle/libs.versions.toml
💤 Files with no reviewable changes (1)
- app/src/main/java/com/smashing/app/MainActivity.kt
🧰 Additional context used
🧬 Code graph analysis (7)
app/src/main/java/com/smashing/app/presentation/dummy/HomeNavigation.kt (1)
app/src/main/java/com/smashing/app/presentation/main/MainAppState.kt (1)
navigate(68-86)
app/src/main/java/com/smashing/app/presentation/home/navigation/HomeNavigation.kt (2)
app/src/main/java/com/smashing/app/presentation/main/MainAppState.kt (1)
navigate(68-86)app/src/main/java/com/smashing/app/presentation/home/HomeScreen.kt (1)
HomeRoute(20-34)
app/src/main/java/com/smashing/app/presentation/home/component/DummyItem.kt (1)
app/src/main/java/com/smashing/app/core/designsystem/component/UrlImage.kt (1)
UrlImage(27-49)
app/src/main/java/com/smashing/app/presentation/home/HomeScreen.kt (1)
app/src/main/java/com/smashing/app/presentation/home/component/DummyItem.kt (1)
DummyUserItem(19-56)
app/src/main/java/com/smashing/app/presentation/main/MainActivity.kt (3)
app/src/main/java/com/smashing/app/core/designsystem/theme/Theme.kt (1)
SmashingAndroidTheme(35-57)app/src/main/java/com/smashing/app/presentation/main/MainAppState.kt (1)
rememberMainAppState(93-99)app/src/main/java/com/smashing/app/presentation/main/MainScreen.kt (1)
MainScreen(19-44)
app/src/main/java/com/smashing/app/presentation/main/MainScreen.kt (3)
app/src/main/java/com/smashing/app/presentation/main/component/MainBottomBar.kt (1)
MainBottomBar(32-71)app/src/main/java/com/smashing/app/presentation/home/navigation/HomeNavigation.kt (1)
homeGraph(18-26)app/src/main/java/com/smashing/app/presentation/dummy/HomeNavigation.kt (1)
dummyGraph(22-38)
app/src/main/java/com/smashing/app/data/repository/impl/DummyRepositoryImpl.kt (1)
app/src/main/java/com/smashing/app/core/util/SuspendRunCatching.kt (1)
suspendRunCatching(18-29)
🔇 Additional comments (44)
app/src/main/res/drawable/ic_fake_red.xml (1)
1-9: 정상적으로 구현된 드로어블 리소스입니다.벡터 드로어블 정의가 올바르고, 40×40 dp 빨간색 정사각형 아이콘이 명확하게 지정되어 있습니다. XML 구문도 정확하고 뷰포트 크기가 치수와 일치합니다.
.github/auto_assign.yml (1)
5-5: 이 구성은 유효합니다.
addAssignees: author는 kentaro-m/auto-assign-action에서 공식적으로 지원하는 유효한 값입니다. 이 설정을 사용하면 PR 작성자가 자동으로 assignee로 추가됩니다.app/src/main/java/com/smashing/app/core/extension/StringExt.kt (1)
5-12: KDoc 문서화가 잘 되어 있습니다.사용자가 인식하는 문자 개수를 반환한다는 설명이 명확합니다. 이모지와 결합 문자 처리에 대한 설명도 포함되어 있어 좋습니다.
app/src/main/java/com/smashing/app/core/local/datastore/di/DataStoreQualifier.kt (1)
1-7: LGTM!Dagger Hilt qualifier 어노테이션이 올바르게 정의되었습니다.
@Qualifier와BINARYretention을 사용한 표준 패턴입니다.app/src/main/java/com/smashing/app/core/local/datastore/di/DataStoreModule.kt (1)
13-25: LGTM!DataStore 모듈이 올바르게 구성되었습니다.
preferencesDataStoredelegate를 사용한 표준 패턴이며,@TokenDataStorequalifier를 통해 DI 그래프에서 구분할 수 있습니다.app/src/main/java/com/smashing/app/data/di/auth/AuthDataSourceModule.kt (1)
11-20: 모듈 구조는 올바르나, 구현체의 생성자 어노테이션 확인 필요
@Binds패턴을 사용한 표준 DI 바인딩입니다. 다만@Binds는 구현체가@Inject생성자를 가져야 Dagger가 인스턴스를 생성할 수 있습니다.LocalTokenDataSourceImpl의 생성자에@Inject어노테이션이 누락된 것으로 보입니다 (별도 코멘트 참조).app/src/main/java/com/smashing/app/data/local/datasource/api/TokenDatasource.kt (1)
3-9: LGTM!토큰 관리를 위한 명확한 인터페이스 계약입니다. 모든 메서드가 DataStore의 비동기 특성에 맞게
suspend함수로 정의되어 있습니다.app/src/main/java/com/smashing/app/data/local/datasource/impl/TokenDatasourceImpl.kt (3)
16-24: LGTM!토큰 읽기 로직이 올바르게 구현되었습니다. DataStore의
dataFlow를map하여 값을 추출하고firstOrNull()로 현재 값을 가져오는 표준 패턴입니다.
26-36: LGTM!토큰 저장 로직이 올바르게 구현되었습니다.
dataStore.edit를 사용하여 Preferences를 안전하게 업데이트하는 표준 패턴입니다.
38-43: LGTM!토큰 삭제 로직이 올바르게 구현되었습니다.
remove()를 사용하여 두 토큰을 모두 제거합니다.app/src/main/java/com/smashing/app/core/extension/FlowExt.kt (1)
9-22: 훌륭한 KDoc 문서화입니다!문서가 매우 상세하며 다음 내용을 잘 설명하고 있습니다:
WhileSubscribed전략의 동작 방식- 기본 timeout 값의 근거 (Configuration change 대응)
- 모든 파라미터와 반환값에 대한 명확한 설명
개발자가 이 함수의 목적과 사용법을 쉽게 이해할 수 있습니다.
app/src/main/java/com/smashing/app/core/common/navigation/Route.kt (1)
1-3: 네비게이션 계층 구조의 기반이 잘 설정되었습니다.마커 인터페이스 패턴을 사용한 타입-안전 네비게이션 구조가 적절합니다.
MainTabRoute및 향후 다른 라우트 타입들의 베이스로 활용될 수 있습니다.app/src/main/java/com/smashing/app/core/common/navigation/MainTabRoute.kt (1)
1-3: 메인 탭 라우트 타입이 올바르게 정의되었습니다.
Route를 확장하여 메인 탭 전용 네비게이션 타입을 분리한 구조가 적절합니다.Home및Dummy네비게이션에서 일관되게 사용될 수 있습니다.app/src/main/java/com/smashing/app/core/designsystem/theme/Color.kt (1)
1-1: 패키지 구조 개선이 적절합니다.테마 관련 파일을
core.designsystem.theme패키지로 이동하여 디자인 시스템 모듈화가 잘 이루어졌습니다. 컬러 값의 기능적 변경은 없습니다.Also applies to: 11-11
app/src/main/java/com/smashing/app/core/designsystem/theme/Type.kt (1)
1-1: 타이포그래피 패키지 재구성이 올바릅니다.
core.designsystem.theme패키지로의 이동이 다른 테마 파일들과 일관성 있게 적용되었습니다. Material 3 Typography 설정에 변경사항이 없습니다.Also applies to: 34-34
app/src/main/java/com/smashing/app/data/model/DummyUser.kt (1)
1-9: 데이터 모델이 명확하게 정의되었습니다.불변(immutable) 속성들로 구성된 깔끔한 데이터 클래스입니다. 프로퍼티 타입이 적절하며, 데이터/도메인/프레젠테이션 레이어에서 일관되게 사용될 수 있는 구조입니다.
app/src/main/res/values/strings.xml (1)
3-6: 문자열 리소스가 올바르게 추가되었습니다.메인 탭에 사용될 한글 문자열 리소스(
홈,더미)가 적절하게 정의되었습니다. 주석을 통한 섹션 구분도 명확합니다.build.gradle.kts (1)
6-9: LGTM! 플러그인 구성이 적절합니다.루트 레벨에서
apply false로 플러그인을 선언하는 것은 멀티모듈 프로젝트의 표준 패턴입니다. 서브모듈에서 필요에 따라 적용할 수 있습니다.app/src/main/java/com/smashing/app/core/designsystem/theme/Theme.kt (1)
1-1: LGTM! 패키지 재구성이 적절합니다.디자인 시스템을
core패키지로 이동하는 것은 클린 아키텍처 관점에서 좋은 접근입니다.app/src/main/java/com/smashing/app/core/util/SuspendRunCatching.kt (1)
18-29: LGTM! 코루틴 취소 시맨틱을 올바르게 처리합니다.
CancellationException을 재throw하고ensureActive()를 호출하는 구현이 정확합니다.TimeoutCancellationException을 실패로 처리하는 것도 적절한 설계 결정입니다.app/src/main/java/com/smashing/app/presentation/dummy/HomeNavigation.kt (1)
18-41: LGTM! 네비게이션 구성이 표준 패턴을 따릅니다.타입-안전 네비게이션을 사용하고 있으며, 더미 화면의 플레이스홀더 구현이 적절합니다.
app/src/main/java/com/smashing/app/core/common/state/UiState.kt (1)
3-15: LGTM! UI 상태 관리 패턴이 적절합니다.Sealed interface를 사용한 타입-안전 상태 관리가 잘 구현되어 있습니다.
Success의 공변성(covariance) 처리도 정확합니다.app/src/main/java/com/smashing/app/presentation/home/navigation/HomeNavigation.kt (1)
14-29: LGTM! 네비게이션 구조가 일관되고 명확합니다.타입-안전 네비게이션을 활용하고 있으며,
innerPadding을HomeRoute에 올바르게 전달하고 있습니다.app/src/main/java/com/smashing/app/presentation/main/MainTab.kt (1)
24-39: 동일한 route를 사용하는 탭들에 대한 주의
DUMMY,DUMMY1,DUMMY2가 모두 동일한Dummyroute를 사용합니다.find함수 호출 시 항상 첫 번째 매칭 탭(DUMMY)만 반환되어 의도치 않은 동작이 발생할 수 있습니다. TODO 주석에 따라 추후 변경 예정이라면 현재 상태에서는 문제없습니다.app/src/main/java/com/smashing/app/presentation/main/MainAppState.kt (1)
93-99: LGTM!
rememberMainAppState함수가 Compose의 상태 홀더 패턴을 올바르게 따르고 있습니다.navController와coroutineScope를 키로 사용하여 적절히 remember 처리되어 있습니다.gradle/libs.versions.toml (1)
108-144: LGTM!번들 정의가 잘 구성되어 있습니다.
androidx-core,compose,network,hilt,debug,test번들로 의존성을 그룹화하여 관리하기 좋습니다.app/src/main/java/com/smashing/app/data/repository/api/DummyRepository.kt (1)
5-7: 잘 구성된 레포지토리 인터페이스입니다.
Result타입을 사용하여 성공/실패를 명시적으로 처리하는 점이 좋습니다. 클린 아키텍처 패턴을 잘 따르고 있습니다.app/src/main/java/com/smashing/app/data/di/dummy/DummyServiceModule.kt (1)
12-18: 표준 Hilt DI 패턴을 잘 따르고 있습니다.Retrofit 서비스 제공을 위한 모듈 구성이 깔끔하고 적절합니다.
@Singleton스코프도 올바르게 적용되었습니다.app/src/main/java/com/smashing/app/data/remote/datasource/api/DummyDataSource.kt (1)
5-7: 깔끔한 데이터소스 인터페이스입니다.레이어 분리가 명확하고 suspend 함수를 사용한 비동기 처리가 적절합니다.
app/src/main/java/com/smashing/app/presentation/main/MainActivity.kt (1)
10-25: Compose 기반 액티비티 구성이 훌륭합니다.
@AndroidEntryPoint로 Hilt 통합enableEdgeToEdge()로 최신 UI 스타일 적용- 상태 관리(
rememberMainAppState)와 화면 구성이 적절하게 분리되어 있습니다현대적인 Android 개발 패턴을 잘 따르고 있습니다.
app/src/main/java/com/smashing/app/data/remote/datasource/impl/DummyDataSourceImpl.kt (1)
8-14: 데이터소스 구현이 적절합니다.서비스 계층으로의 단순 위임이 명확하며, 에러 처리는 상위 레이어(repository)에서
suspendRunCatching으로 처리되는 구조가 적절합니다.app/src/main/java/com/smashing/app/presentation/home/HomeContract.kt (1)
8-14: 우수한 상태 관리 패턴입니다.
@Immutable과ImmutableList사용으로 불변성 보장UiState타입으로 로딩/성공/실패 상태를 명확히 표현- MVI 아키텍처 패턴을 잘 따르고 있습니다
Compose의 재구성 최적화에도 유리한 구조입니다.
app/src/main/java/com/smashing/app/data/di/dummy/DummyRepositoryModule.kt (1)
11-19: Hilt 바인딩 모듈이 올바르게 구성되었습니다.
@Binds를 사용한 인터페이스-구현체 바인딩이 효율적이고 적절합니다. DI 아키텍처가 깔끔하게 설계되었습니다.app/src/main/java/com/smashing/app/data/di/dummy/DummyDataSourceModule.kt (1)
11-19: LGTM!Hilt 바인딩 모듈이 올바르게 구성되어 있습니다.
@Binds를 사용한 추상 메서드 패턴과@Singleton스코프 적용이 적절합니다.app/src/main/java/com/smashing/app/presentation/main/MainScreen.kt (1)
19-44: LGTM!
MainScreen구조가 잘 설계되어 있습니다.collectAsStateWithLifecycle을 사용한 라이프사이클 인식 상태 수집과Scaffold구성이 적절합니다.app/src/main/java/com/smashing/app/data/repository/impl/DummyRepositoryImpl.kt (1)
10-18: LGTM!리포지토리 구현이 깔끔합니다.
suspendRunCatching을 활용한 에러 핸들링과 null-safe 매핑 처리가 적절합니다.app/src/main/java/com/smashing/app/SmashingApplication.kt (1)
8-24: 다크 모드 지원 의도 확인애플리케이션 초기화가 올바르게 구성되어 있습니다. 다만 Line 22에서
MODE_NIGHT_NO로 설정하여 다크 모드를 비활성화하고 있습니다. 이것이 의도된 정책인지 확인하시기 바랍니다. 최신 안드로이드 가이드라인에서는 다크 모드 지원을 권장합니다.app/src/main/java/com/smashing/app/presentation/home/HomeScreen.kt (2)
20-34: LGTM!
HomeRoute구성이 우수합니다. Hilt를 통한 ViewModel 주입,collectAsStateWithLifecycle을 사용한 상태 수집, 그리고LaunchedEffect(Unit)을 활용한 초기 데이터 페치가 모두 Compose 베스트 프랙티스를 따르고 있습니다.
67-86: LGTM!
LazyColumn에서key파라미터를 사용하여 리컴포지션 최적화를 적용한 것이 좋습니다. 리스트 아이템 렌더링이 적절하게 구성되어 있습니다.app/src/main/AndroidManifest.xml (1)
3-6: LGTM!
INTERNET권한과 커스텀 Application 클래스 설정이 올바르게 구성되어 있습니다.app/src/main/java/com/smashing/app/presentation/home/HomeViewModel.kt (1)
17-23: LGTM!ViewModel 구조가 올바르게 설계되어 있습니다. Hilt를 통한 의존성 주입과
StateFlow를 활용한 상태 관리가 적절합니다.app/src/main/java/com/smashing/app/data/remote/service/DummyService.kt (1)
8-14: 테스트용 더미 서비스이므로 코드 수정이 불필요합니다.DummyService는 reqres.in의 공개 테스트 API를 위한 더미 서비스입니다. "reqres-free-v1"은 reqres.in의 무료/데모 계층을 위한 공식 문서화된 API 키이며, 공개된 테스트 API 키이므로 보안 문제가 아닙니다. 서비스 이름과
dummy모듈 경로는 이것이 테스트 목적 코드임을 명확히 나타냅니다.Likely an incorrect or invalid review comment.
app/src/main/java/com/smashing/app/data/remote/dto/GetUserListResponse.kt (2)
15-22: UserData 구조가 올바르게 정의되었습니다.중첩 데이터 클래스 구조가 적절하며, API 필드명(
avatar)을 의미론적으로 더 명확한 Kotlin 프로퍼티명(profileImage)으로 매핑한 것은 좋은 선택입니다.
6-14: DTO 구조가 적절하게 설계되었습니다.API 응답 역직렬화를 위한 구조가 올바르게 구성되어 있습니다:
- 모든 필드에
@SerialName어노테이션을 사용하여 API 계약 안정성을 확보했습니다.- 외부 API 응답의 불확실성을 고려하여 모든 필드를 nullable로 처리한 것은 적절한 방어 코딩입니다.
DummyMapper.kt에서 Elvis 연산자(?:)와orEmpty()를 사용하여 null 케이스를 안전하게 처리하고 있습니다.
app/build.gradle.kts
Outdated
|
|
||
| implementation(libs.timber) | ||
| implementation(libs.lottie.compose) | ||
| ksp(libs.kotlin.metadata.jvm) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for kotlin-metadata usage in gradle files
rg "kotlin-metadata" --type=gradle -nRepository: TEAM-SMASHING/SMASHING-ANDROID
Length of output: 222
🏁 Script executed:
#!/bin/bash
# Check the full build.gradle.kts file around line 92 for context
head -100 app/build.gradle.kts | tail -20Repository: TEAM-SMASHING/SMASHING-ANDROID
Length of output: 538
🏁 Script executed:
#!/bin/bash
# Search for ksp declarations and their usage
rg "ksp\(" --type=gradle -B2 -A2Repository: TEAM-SMASHING/SMASHING-ANDROID
Length of output: 222
🏁 Script executed:
#!/bin/bash
# Find gradle files
find . -name "*.gradle.kts" -o -name "*.gradle" -o -name "*.toml" | head -20Repository: TEAM-SMASHING/SMASHING-ANDROID
Length of output: 165
🏁 Script executed:
#!/bin/bash
# Search for kotlin-metadata in all files without type filter
rg "kotlin-metadata" -nRepository: TEAM-SMASHING/SMASHING-ANDROID
Length of output: 213
🏁 Script executed:
#!/bin/bash
# Check the full app/build.gradle.kts to understand the dependency structure
wc -l app/build.gradle.ktsRepository: TEAM-SMASHING/SMASHING-ANDROID
Length of output: 97
🏁 Script executed:
#!/bin/bash
# Read the full app/build.gradle.kts file
cat -n app/build.gradle.ktsRepository: TEAM-SMASHING/SMASHING-ANDROID
Length of output: 3220
🏁 Script executed:
#!/bin/bash
# Search for kotlin.metadata usage in source code
rg "kotlin.metadata" -n --type kotlinRepository: TEAM-SMASHING/SMASHING-ANDROID
Length of output: 131
🏁 Script executed:
#!/bin/bash
# Check if there are any KSP plugins or configurations
rg "plugin.*ksp\|id.*ksp" -n --type kotlinRepository: TEAM-SMASHING/SMASHING-ANDROID
Length of output: 56
🏁 Script executed:
#!/bin/bash
# Check gradle/libs.versions.toml for more context on the dependency
cat gradle/libs.versions.tomlRepository: TEAM-SMASHING/SMASHING-ANDROID
Length of output: 5307
🏁 Script executed:
#!/bin/bash
# Search for any Hilt or other annotation processor that might require kotlin-metadata-jvm
rg "Hilt\|kapt\|ksp" app/build.gradle.kts -B3 -A3Repository: TEAM-SMASHING/SMASHING-ANDROID
Length of output: 56
kotlin-metadata-jvm을 KSP 프로세서로 사용하는 것은 잘못된 구성입니다
kotlin-metadata-jvm은 Kotlin 메타데이터에 대한 런타임 리플렉션을 위한 라이브러리로, KSP 애노테이션 프로세서가 아닙니다. 이 의존성이 필요하면 implementation(libs.kotlin.metadata.jvm)으로 변경하거나, 사용하지 않는다면 제거해야 합니다. 현재 Hilt만이 유일한 KSP 프로세서입니다.
🤖 Prompt for AI Agents
In app/build.gradle.kts around line 92, the dependency
ksp(libs.kotlin.metadata.jvm) is incorrect because kotlin-metadata-jvm is not a
KSP processor; replace that ksp(...) entry with
implementation(libs.kotlin.metadata.jvm) if you need the library at runtime, or
remove the dependency entirely if unused, and ensure only actual KSP processors
(e.g., Hilt) remain under ksp.
| val loading by rememberUpdatedState(isLoading) | ||
|
|
||
| val shouldLoadMore by remember { | ||
| derivedStateOf { | ||
| val total = layoutInfo.totalItemsCount | ||
| if (total == 0 || isLoading) return@derivedStateOf false | ||
|
|
||
| val lastVisible = layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: -1 | ||
| lastVisible + threshold >= total - 1 | ||
| } | ||
| } |
There was a problem hiding this comment.
loading 변수 미사용 - 캡처된 값 불일치 가능성
rememberUpdatedState로 loading 변수를 생성했지만, derivedStateOf 블록에서 isLoading 파라미터를 직접 참조하고 있습니다. 이로 인해 loading 변수가 사용되지 않고, 상태 업데이트가 올바르게 반영되지 않을 수 있습니다.
🔎 수정 제안
val shouldLoadMore by remember {
derivedStateOf {
val total = layoutInfo.totalItemsCount
- if (total == 0 || isLoading) return@derivedStateOf false
+ if (total == 0 || loading) return@derivedStateOf false
val lastVisible = layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: -1
lastVisible + threshold >= total - 1
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| val loading by rememberUpdatedState(isLoading) | |
| val shouldLoadMore by remember { | |
| derivedStateOf { | |
| val total = layoutInfo.totalItemsCount | |
| if (total == 0 || isLoading) return@derivedStateOf false | |
| val lastVisible = layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: -1 | |
| lastVisible + threshold >= total - 1 | |
| } | |
| } | |
| val loading by rememberUpdatedState(isLoading) | |
| val shouldLoadMore by remember { | |
| derivedStateOf { | |
| val total = layoutInfo.totalItemsCount | |
| if (total == 0 || loading) return@derivedStateOf false | |
| val lastVisible = layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: -1 | |
| lastVisible + threshold >= total - 1 | |
| } | |
| } |
🤖 Prompt for AI Agents
In app/src/main/java/com/smashing/app/core/extension/LazyListStateExt.kt around
lines 32 to 42, the code creates a rememberUpdatedState called loading but still
reads isLoading directly inside derivedStateOf, which can cause stale capture
issues; replace references to isLoading inside the derivedStateOf block with the
loading value provided by rememberUpdatedState (or remove rememberUpdatedState
and consistently use isLoading), ensuring the derivedStateOf reads the stable,
up-to-date value so state updates propagate correctly.
| class LocalTokenDataSourceImpl( | ||
| @TokenDataStore private val dataStore: DataStore<Preferences>, | ||
| ) : LocalTokenDataSource { |
There was a problem hiding this comment.
생성자에 @Inject 어노테이션 누락
AuthDataSourceModule에서 @Binds를 사용하여 이 구현체를 바인딩하는데, @Binds는 구현 클래스가 @Inject 어노테이션이 있는 생성자를 가져야 Dagger가 인스턴스를 생성할 수 있습니다. 현재 생성자에 @Inject가 없어 컴파일 오류가 발생하거나 런타임에 DI가 실패할 수 있습니다.
🔎 생성자에 @Inject 추가
+import javax.inject.Inject
+
-class LocalTokenDataSourceImpl(
+class LocalTokenDataSourceImpl @Inject constructor(
@TokenDataStore private val dataStore: DataStore<Preferences>,
) : LocalTokenDataSource {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| class LocalTokenDataSourceImpl( | |
| @TokenDataStore private val dataStore: DataStore<Preferences>, | |
| ) : LocalTokenDataSource { | |
| import javax.inject.Inject | |
| class LocalTokenDataSourceImpl @Inject constructor( | |
| @TokenDataStore private val dataStore: DataStore<Preferences>, | |
| ) : LocalTokenDataSource { |
🤖 Prompt for AI Agents
In
app/src/main/java/com/smashing/app/data/local/datasource/impl/TokenDatasourceImpl.kt
around lines 12 to 14, the constructor is missing the @Inject annotation
required for Dagger when using @Binds; add @Inject to the primary constructor
(e.g., annotate the constructor that receives @TokenDataStore
DataStore<Preferences>) and import the correct Inject type (javax.inject.Inject
or jakarta.inject.Inject consistent with the project), then rebuild to ensure
Dagger can construct and bind this implementation.
app/src/main/java/com/smashing/app/data/remote/dto/GetUserListResponse.kt
Show resolved
Hide resolved
app/src/main/java/com/smashing/app/presentation/main/component/MainBottomBar.kt
Show resolved
Hide resolved
oilbeaneda
left a comment
oilbeaneda
left a comment
There was a problem hiding this comment.
리드님 넘 수고하셨습니다 🤩아자스!!!!!
| import androidx.compose.ui.composed | ||
|
|
||
| /** | ||
| * 리플 효과 없이 클릭 가능하게 만드는 Modifier |
There was a problem hiding this comment.
보통 리플효과 없이 클릭 or 있게 클릭 여부는 개발 단계에서 디자이너 분들이 정해주시는지 아니면 프론트에서 ux를 고려해서 정하는지 궁금합니다!
| fun fetchDummyUsers() = viewModelScope.launch { | ||
| dummyRepository.fetchDummyUserList(page = 1).onSuccess { userList -> |
There was a problem hiding this comment.
loading 상태를 추가하려면
| fun fetchDummyUsers() = viewModelScope.launch { | |
| dummyRepository.fetchDummyUserList(page = 1).onSuccess { userList -> | |
| fun fetchDummyUsers() = viewModelScope.launch { | |
| updateDummyUiState(UiState.Loading) | |
| dummyRepository.fetchDummyUserList(page = 1).onSuccess { userList -> |
여기에 이런식으로 쓰는게 맞을까요?
There was a problem hiding this comment.
네 맞습니다 ㅎ.. Loading 부분 추가 되어야하네용
| * @param timeout 구독 해제 후 업스트림 취소까지의 대기 시간 (기본: 5000ms) | ||
| * @return Hot StateFlow | ||
| */ | ||
| fun <T> Flow<T>.stateInWhileSubscribed( |
There was a problem hiding this comment.
상태 값 연산은 cold Flow로 처리하고 최종 값을 hot Flow로 변환해서 상태를 공유할 수 있도록 하되 SharingStarted.WhileSubscribed를 통해 구독자가 없을 때만 업스트림 collect를 중단하는 걸로 이해했는데.. 이게 맞을까요?
There was a problem hiding this comment.
state in 같은 경우는 collect(구독자) 가 없는 경우에 원본(업스트림) flow 에 대해서 cancel 을 수행한다고 보시면 됩니다 !
만약, ui 에서 collect 하고 있는 경우에 화면회전이 일어나면 collect 는 제거 되어서 일반적인 경우라면 collect 가 취소되었기에 다시 flow 를 수행하겠지만, state in 에서 time out을 사용하면 flow 를 처음부터 다시 시작하지 않고 5초 이내로 flow 를 유지합니다. 이 5초의 의미는 메인스레드가 block 할 수 있는 최대 시간이 5초이기에 해당 시간으로 설정했습니다.
++어제 이야기 하던 걸 마저 하자면
cold flow는 collect 호출마다 새로 실행됩니다. 따라서, 그대로 사용하게 될 경우 각각의 collect 를 수행하는 곳에서 중복 실행이 됩니다. 예를 들어 api 호출 하는 로직을 다른곳에서 동시에 collect 하고 있다한다면 api 호출은 동시에 두군데에서 일어나게 되는 것이죠.
이와 반대로 hot flow 는 하나의 flow 를 가지고 여러 곳에서 collect 하고 공유하기 때문에 api 호출은 한번만 일어나고 각각의 emit 으로 받은 상태값을 사용해 Ui 를 업데이트 합니다.
다음 링크들을 참고하면 더 좋을거 같아요.
android stateIn
stateIn 관련 설명글
ShinHyeongcheol
left a comment
ShinHyeongcheol
left a comment
There was a problem hiding this comment.
기본 세팅 수고하셨습니다!!
중간에 이해한게 맞는지 코멘트 하나 남겼어요.
| private val _uiState = MutableStateFlow(HomeContract.State()) | ||
| val uiState = _uiState.asStateFlow() |
There was a problem hiding this comment.
공통 UiState나 Contract를 사용하지 않아서 이 부분 질문드리는데,
SideEffect와 같은 경우에는 HomeContract에 sealed class Effect{}를 만들고
private val _effect = MutableSharedFlow<HomeContract.Effect>처럼 사용하면 되는걸까요?
There was a problem hiding this comment.
네 저번 1차 회의 떄 말씀 드렸던 패턴을 사용하는데요!
아래 링크 노션에 적혀있는 부분을 봐주시면 더 좋을거 같습니다 !
노션 링크
Related issue 🛠
Work Description ✏️
Screenshot 📸
Screen_recording_20251231_183227.mp4
Uncompleted Tasks 😅
To Reviewers 📢
질문 or 반박 환영입니다. 전체적인 구조에 대해서 봐주시면 좋을거 같습니다.
유틸 함수들 필요할만한거 추가해서 kdoc 작성해놓았는데 활용하시면 됩니다 !!
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선 사항
✏️ Tip: You can customize this high-level summary in your review settings.