🔀 :: (#54) MainScreen 에 통신한 것을 적용위한 리펙토링을 했습니다. #60
Conversation
Walkthrough이 PR은 Jetpack Compose 기반 주요 화면과 관련 컴포저블 함수들의 기능을 개선하고, UI 상태 관리 및 네비게이션, 오류 처리 로직을 업데이트합니다. 주요 변경 사항으로는 MainScreen.kt에서 상태 전달 방식과 콜백 호출 추가, 새로운 MainRoute 컴포저블 도입, MyRanking 및 RankingList 함수의 추가, 그리고 뷰모델과 상태 클래스(Fail 상태)의 예외 정보 포함 등이 있습니다. 또한, 네비게이션 관련 MainNavigation.kt와 문자열 리소스(string.xml)에도 변경이 이루어졌습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Nav as NavController
participant Main as MainScreen/MainRoute
participant VM as HomesViewModel
participant UI as UI Components
Nav->>Main: navigationToMain() 호출
Main->>VM: collectAsStateWithLifecycle로 UI 상태 수집
VM-->>Main: 상태 전송 (Loading/Success/Fail)
Main->>UI: MyRanking, RankingList 등 컴포저블 실행
UI->>Main: 오류 발생 시 onErrorToast 콜백 호출
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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[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 (5)
presentation/src/main/res/values/string.xml (1)
7-8: 신규 오류 문자열 리소스 추가됨
새로운 문자열 리소스<string name="error">사용자 정보를 찾을수 없습니다</string>가 추가되었습니다. 해당 문자열은 MainScreen에서 오류 처리시 사용자에게 전달될 메시지로 적절해 보입니다.
추가로 "찾을수" 보다 "찾을 수"와 같이 띄어쓰기를 적용하면 가독성이 더 좋아질 수 있으므로 검토해 보시길 권장합니다.presentation/src/main/java/viewModel/homes/HomesViewmodel.kt (2)
39-40: 불필요한 빈 줄을 제거해주세요.코드의 가독성을 위해 40번 줄의 불필요한 빈 줄을 제거하는 것이 좋겠습니다.
61-61: 오류 메시지 처리 방식을 개선해주세요.
HomesMyRankUiState.Fail의 message 매개변수에 null을 하드코딩하는 대신, 의미 있는 기본 오류 메시지를 제공하는 것이 좋겠습니다.예시:
- _homesMyRankUiState.value = HomesMyRankUiState.Fail(result.exception,null) + _homesMyRankUiState.value = HomesMyRankUiState.Fail( + throwable = result.exception, + message = R.string.error_failed_to_load_rank + )presentation/src/main/java/view/main/component/Rankingcomponent.kt (2)
35-63: UI 상태 처리 개선이 필요합니다.다음 사항들을 개선하면 좋을 것 같습니다:
- Empty 상태에 대한 처리가 누락되어 있습니다.
- Loading 상태가 단순 텍스트로만 표시되고 있습니다.
다음과 같이 개선하는 것을 제안드립니다:
HomesUiState.Loading -> { - Text(text = "로딩중") + CircularProgressIndicator( + modifier = Modifier.align(Alignment.CenterHorizontally) + ) } is HomesUiState.Empty -> { + Text( + text = stringResource(R.string.no_ranking_data), + modifier = Modifier.align(Alignment.CenterHorizontally), + style = MaterialTheme.typography.bodyLarge + ) }
90-90: 문자열 리소스 사용을 권장드립니다.순위와 점수 표시에 사용되는 접미사("위", "점")를 strings.xml 리소스로 분리하면 좋을 것 같습니다.
다음과 같이 수정하는 것을 제안드립니다:
-text = "${data.rank}위" +text = stringResource(R.string.rank_format, data.rank) -text = "${data.penaltyPoint}점" +text = stringResource(R.string.points_format, data.penaltyPoint)strings.xml에 다음 내용 추가:
<string name="rank_format">%d위</string> <string name="points_format">%d점</string>Also applies to: 108-108
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
presentation/src/main/java/view/main/Screen/MainScreen.kt(5 hunks)presentation/src/main/java/view/main/component/MyRankingComponent.kt(2 hunks)presentation/src/main/java/view/main/component/Rankingcomponent.kt(5 hunks)presentation/src/main/java/view/main/navigation/MainNavigation.kt(1 hunks)presentation/src/main/java/viewModel/homes/HomesViewmodel.kt(2 hunks)presentation/src/main/java/viewModel/homes/uistate/HomesMyRankUiState.kt(1 hunks)presentation/src/main/java/viewModel/homes/uistate/HomesUiState.kt(1 hunks)presentation/src/main/res/values/string.xml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (14)
presentation/src/main/java/viewModel/homes/uistate/HomesUiState.kt (1)
9-9: 예외 처리 개선을 위한 Fail 상태 리팩토링이 잘 되었습니다.예외 정보를 포함하도록 Fail 상태를 개선한 것이 좋습니다. 이를 통해 사용자에게 더 자세한 오류 정보를 제공할 수 있게 되었습니다.
presentation/src/main/java/viewModel/homes/uistate/HomesMyRankUiState.kt (1)
9-9: Fail 상태의 매개변수에 대한 검토가 필요합니다.Fail 상태에 nullable 매개변수들을 사용하고 있습니다:
- throwable이 null일 때의 처리 방식을 명확히 해야 합니다
- message의 용도와 null일 때의 기본값을 문서화하면 좋겠습니다
다음과 같이 개선하는 것을 고려해보세요:
- data class Fail(val throwable: Throwable?, val message: Int?) : HomesMyRankUiState + data class Fail( + val throwable: Throwable? = null, + @StringRes val message: Int? = null + ) : HomesMyRankUiStatepresentation/src/main/java/view/main/Screen/MainScreen.kt (6)
3-3: 모듈화된 임포트 구성이 잘 이루어졌습니다.
별다른 문제 없이 필요한 라이브러리를 적절히 불러오고 있습니다.Also applies to: 17-18, 22-22, 28-29, 31-32, 34-36
38-58:MainRoute함수 설계가 적절합니다.
MainScreen호출 전에 ViewModel 상태를 수집하고 주입하여, UI와 로직 사이의 분리가 명료해졌습니다.
63-68:MainScreen파라미터 확장
homesUiState,homesMyRankUiState등 상태를 직접 주입받아 관리하는 방식으로, UI 상태 예외 처리가 용이해졌습니다.
104-107:MyRanking컴포저블 사용
homesMyRankUiState를 직접 전달하며, 에러 콜백도 연결되어 있어 UI 상태 관리가 간결해졌습니다.
172-175:RankingList컴포저블 사용
homesUiState를 받아 내부에서 다양한 상태 처리를 수행하도록 위임한 구조가 명확합니다.
186-192: 프리뷰 함수 파라미터 예시가 적절합니다.
프리뷰에서 Loading 상태를 시뮬레이션하여 UI 확인이 깔끔해졌습니다.presentation/src/main/java/view/main/navigation/MainNavigation.kt (1)
1-25: 신규 네비게이션 구성 코드가 명확합니다.
navigationToMain()확장 함수로 단순화된 라우팅이 가능합니다.mainScreen에서composable누적 및MainRoute호출 흐름이 자연스럽습니다.presentation/src/main/java/view/main/component/MyRankingComponent.kt (4)
23-23: 임포트 추가
MyRankResponseModel과HomesMyRankUiState불러오는 부분에 문제 없습니다.Also applies to: 25-25
27-54: 새로 추가된MyRanking함수
when블록을 통해 로딩, 성공, 실패, 빈 상태를 구분하여 적절히 처리하고 있습니다. 구조가 간결하고 가독성이 높습니다.
62-62:MyRankingComponent파라미터 변경
기존 정수rank대신 모델(MyRankResponseModel)을 넘겨받아 확장성 있는 코드 구조가 되었습니다.
91-91:data.rank사용
랭킹 값을data객체에서 바로 꺼내 사용하는 방식이 읽기 좋아졌습니다.presentation/src/main/java/view/main/component/Rankingcomponent.kt (1)
12-16: 필요한 import 문이 잘 추가되었습니다!LazyColumn과 PersistentList 관련 import가 적절하게 추가되었습니다.
Also applies to: 27-32
| LaunchedEffect(Unit) { | ||
| myRankCallBack | ||
| rankListCallBack | ||
| } |
There was a problem hiding this comment.
함수 호출 누락으로 인한 로직 오작동 가능성
현재 콜백 함수를 참조만 하고 실제로 호출하지 않아, 예상한 동작(데이터 로딩)이 수행되지 않을 수 있습니다.
아래 diff를 적용하여 콜백을 실제 호출하는 것이 좋습니다:
-LaunchedEffect(Unit) {
- myRankCallBack
- rankListCallBack
-}
+LaunchedEffect(Unit) {
+ myRankCallBack()
+ rankListCallBack()
+}📝 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.
| LaunchedEffect(Unit) { | |
| myRankCallBack | |
| rankListCallBack | |
| } | |
| LaunchedEffect(Unit) { | |
| myRankCallBack() | |
| rankListCallBack() | |
| } |
| @Composable | ||
| internal fun RankingListItem( | ||
| modifier: Modifier = Modifier, | ||
| list: PersistentList<RankResponseModel>, | ||
|
|
||
| ) { | ||
| Column(modifier = modifier.fillMaxWidth()) { | ||
| LazyColumn( | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .heightIn(max = 10_000.dp) | ||
| ) { | ||
| items( | ||
| items = list, | ||
| key = { data -> | ||
| data.userId | ||
| } | ||
| ) { data -> | ||
| RankingComponent( | ||
| modifier = Modifier.fillMaxWidth(), | ||
| data = data | ||
| ) | ||
| } | ||
|
|
||
| } | ||
|
|
||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
LazyColumn의 높이 제한을 재검토해주세요.
현재 설정된 heightIn(max = 10_000.dp)는 다음과 같은 문제가 있을 수 있습니다:
- 불필요하게 큰 값으로 설정되어 있습니다
- 메모리 사용량에 영향을 줄 수 있습니다
다음과 같이 수정하는 것을 제안드립니다:
LazyColumn(
modifier = Modifier
- .fillMaxWidth()
- .heightIn(max = 10_000.dp)
+ .fillMaxWidth()
+ .heightIn(max = 300.dp)
) {또는 스크롤이 필요한 경우라면:
LazyColumn(
modifier = Modifier
- .fillMaxWidth()
- .heightIn(max = 10_000.dp)
+ .fillMaxWidth()
+ .weight(1f)
) {📝 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.
| @Composable | |
| internal fun RankingListItem( | |
| modifier: Modifier = Modifier, | |
| list: PersistentList<RankResponseModel>, | |
| ) { | |
| Column(modifier = modifier.fillMaxWidth()) { | |
| LazyColumn( | |
| modifier = Modifier | |
| .fillMaxWidth() | |
| .heightIn(max = 10_000.dp) | |
| ) { | |
| items( | |
| items = list, | |
| key = { data -> | |
| data.userId | |
| } | |
| ) { data -> | |
| RankingComponent( | |
| modifier = Modifier.fillMaxWidth(), | |
| data = data | |
| ) | |
| } | |
| } | |
| } | |
| } | |
| @Composable | |
| internal fun RankingListItem( | |
| modifier: Modifier = Modifier, | |
| list: PersistentList<RankResponseModel>, | |
| ) { | |
| Column(modifier = modifier.fillMaxWidth()) { | |
| LazyColumn( | |
| modifier = Modifier | |
| .fillMaxWidth() | |
| .heightIn(max = 300.dp) | |
| ) { | |
| items( | |
| items = list, | |
| key = { data -> | |
| data.userId | |
| } | |
| ) { data -> | |
| RankingComponent( | |
| modifier = Modifier.fillMaxWidth(), | |
| data = data | |
| ) | |
| } | |
| } | |
| } | |
| } |
| @Composable | |
| internal fun RankingListItem( | |
| modifier: Modifier = Modifier, | |
| list: PersistentList<RankResponseModel>, | |
| ) { | |
| Column(modifier = modifier.fillMaxWidth()) { | |
| LazyColumn( | |
| modifier = Modifier | |
| .fillMaxWidth() | |
| .heightIn(max = 10_000.dp) | |
| ) { | |
| items( | |
| items = list, | |
| key = { data -> | |
| data.userId | |
| } | |
| ) { data -> | |
| RankingComponent( | |
| modifier = Modifier.fillMaxWidth(), | |
| data = data | |
| ) | |
| } | |
| } | |
| } | |
| } | |
| @Composable | |
| internal fun RankingListItem( | |
| modifier: Modifier = Modifier, | |
| list: PersistentList<RankResponseModel>, | |
| ) { | |
| Column(modifier = modifier.fillMaxWidth()) { | |
| LazyColumn( | |
| modifier = Modifier | |
| .fillMaxWidth() | |
| .weight(1f) | |
| ) { | |
| items( | |
| items = list, | |
| key = { data -> | |
| data.userId | |
| } | |
| ) { data -> | |
| RankingComponent( | |
| modifier = Modifier.fillMaxWidth(), | |
| data = data | |
| ) | |
| } | |
| } | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
presentation/src/main/java/view/main/Screen/MainScreen.kt (2)
77-178: UI 컴포넌트 구조 개선 제안현재 중첩된 레이아웃 구조가 복잡하여 유지보수가 어려울 수 있습니다. 다음과 같은 개선을 제안합니다:
- 각 섹션을 별도의 컴포저블 함수로 분리
- 공통 스타일을 테마로 추출
- 하드코딩된 색상값을 Color 클래스로 분리
예시:
@Composable private fun RankingHeader() { Column( modifier = Modifier.fillMaxWidth(), horizontalAlignment = Alignment.CenterHorizontally ) { Text( text = "이번주 랭킹", style = MaterialTheme.typography.titleLarge.copy( fontWeight = FontWeight.Bold ) ) } }
182-194: 프리뷰 함수 개선 제안현재 프리뷰가 Loading 상태만 보여주고 있습니다. 다양한 상태를 테스트할 수 있도록 개선이 필요합니다.
다음과 같이 여러 상태의 프리뷰를 추가하는 것을 제안합니다:
@Preview(name = "로딩 상태") @Composable fun PreviewMainScreenLoading() { MainScreen( homesUiState = HomesUiState.Loading, // ... other params ) } @Preview(name = "성공 상태") @Composable fun PreviewMainScreenSuccess() { MainScreen( homesUiState = HomesUiState.Success( listOf(/* 테스트 데이터 */) ), // ... other params ) } @Preview(name = "에러 상태") @Composable fun PreviewMainScreenError() { MainScreen( homesUiState = HomesUiState.Fail(Exception("테스트 에러")), // ... other params ) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
presentation/src/main/java/view/main/Screen/MainScreen.kt(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (2)
presentation/src/main/java/view/main/Screen/MainScreen.kt (2)
42-42: LocalContext의 ComponentActivity 캐스팅에 대한 안전성 검토 필요
LocalContext.current as ComponentActivity캐스팅은 런타임 시 ClassCastException을 발생시킬 수 있습니다. 컨텍스트가 항상 ComponentActivity임을 보장할 수 없기 때문입니다.다음과 같은 안전한 방식으로 수정하는 것을 고려해보세요:
-viewModel: HomesViewModel = hiltViewModel(LocalContext.current as ComponentActivity) +viewModel: HomesViewModel = hiltViewModel()
71-74: 콜백 구현이 올바르게 수정되었습니다LaunchedEffect 내에서 콜백 함수들이 적절하게 호출되도록 구현되어 있습니다.
💡 개요
MainScreen 에 통신한 것을 적용위한 리펙토링을 했습니다.
📃 작업내용
MainScreen 에 통신한 것을 적용위한 리펙토링을 했습니다.
🔀 변경사항
RankingComponet
MyRankingComponet
MainNavigation
MainScreen
🙋♂️ 질문사항
🍴 사용방법
🎸 기타
Summary by CodeRabbit
New Features
Refactor
Chores