[API] 모집중인 모임방 및 모임방 화면 API 연결#69
Conversation
| fun provideGroupService(retrofit: Retrofit): GroupService { | ||
| return retrofit.create(GroupService::class.java) | ||
| } |
There was a problem hiding this comment.
오히려 좋을지도... 충돌은 안날거 같네요
There was a problem hiding this comment.
닉네임을 보내주는 API도 있고 안보내주는 API도 있어서 한번 받으면 캐싱해두고 사용하려고 했습니다!
There was a problem hiding this comment.
이게 장르를 관리하는 코드긴한데 "과학·IT" -> "과학/IT" 이거 처리하려고 만든 코드인데 겸사겸사 장르까지 관리하려고 헀습니다
There was a problem hiding this comment.
같은 컴포저블 함수랑 데이터 클래스를 최대한 공유해서 쓰려고 하다 보니까 이런식으로 매핑 코드를 넣게 됬습니다. 이렇게 하는게 좀 비효율적인거 같으면 dto로 바로 사용하는 방식으로 수정해볼게요
There was a problem hiding this comment.
비슷해요 요청하고 BaseResponse처리하고 성공하면 데이터 처리하는건데 Mapper랑 Manager 코드가 들어가서 매핑하는 로직이 추가되서 좀 복잡해 보이는겁니다! 일단 runCatchint으로 수정할게요
| author: String, | ||
| publisher: String, | ||
| imageRes: Int? = R.drawable.bookcover_sample, // 기본 이미지 리소스 | ||
| imageUrl: String? = null, // API에서 받은 이미지 URL |
There was a problem hiding this comment.
그런 말이 없어서 일단 nullable하게 처리하려고 했습니다
| fun GroupDoneScreen( | ||
| onNavigateBack: () -> Unit = {}, | ||
| viewModel: GroupDoneViewModel = hiltViewModel() | ||
| ) { |
| } | ||
| } | ||
| AsyncImage( | ||
| model = imageUrl ?: R.drawable.bookcover_sample, |
There was a problem hiding this 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: 1
♻️ Duplicate comments (8)
app/src/main/java/com/texthip/thip/ui/group/myroom/viewmodel/GroupMyViewModel.kt (1)
63-64: 에러 메시지 개선을 고려해보세요.현재는
exception.message만을 UI 상태에 저장하고 있습니다. 사용자 친화적인 에러 메시지나 에러 타입별 처리를 위해 더 구조화된 에러 핸들링을 고려해보세요.app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt (2)
27-31: 매직 넘버를 상수로 추출 권장페이지네이션 관련 값들이 하드코딩되어 있습니다. 가독성과 유지보수를 위해 companion object의 상수로 추출하는 것이 좋습니다.
companion object { private const val PAGES_PER_BATCH = 3 private const val PRELOAD_THRESHOLD = 2 private const val ITEMS_PER_PAGE = 3 } private var currentMyGroupsPage = 1 private var loadedPagesCount = 0 private val pagesPerBatch = PAGES_PER_BATCH private val preloadThreshold = PRELOAD_THRESHOLD
106-114: onCardVisible의 magic number(3) 제거 및 계산식 상수화 필요현재
onCardVisible에서 페이지당 아이템 수를 나타내는 숫자 3을 하드코딩하여 사용하고 있습니다. 이 값이 API 호출 시 실제 페이지 사이즈 또는 UI에 표시되는 카드 개수와 불일치할 위험이 있으며, 유지보수를 어렵게 만듭니다.companion object에
CARDS_PER_PAGE = 3상수를 추가하고 계산식을(cardIndex / CARDS_PER_PAGE) + 1로 변경하세요.app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt (4)
140-142: 하드코딩된 문자열이 string resource로 잘 변경되었습니다.이전 리뷰에서 지적된 하드코딩된 문자열들이 적절히 string resource로 추출되었습니다. 유지보수성과 현지화 지원이 개선되었습니다.
Also applies to: 146-147
170-170: 에러 메시지의 string resource 적용이 잘 되었습니다.네트워크 오류 및 모임방 생성 실패 메시지도 string resource를 사용하도록 적절히 변경되었습니다.
Also applies to: 173-173
81-83: 예외 처리에 로깅 추가가 필요합니다.catch 블록에서 예외를 삼키고 있어 디버깅이 어려울 수 있습니다. 프로덕션 환경에서 문제 추적을 위해 로깅을 추가하는 것을 권장합니다.
다음과 같이 수정하세요:
} catch (e: Exception) { + // TODO: 로깅 프레임워크 추가 후 적용 + // Log.e("GroupMakeRoomViewModel", "Failed to load books", e) updateState { it.copy(savedBooks = emptyList(), groupBooks = emptyList()) } }
180-190: 장르 매핑 로직에 중복이 있습니다.
GenreManager에서 이미mapGenreToApiCategory메서드를 제공하고 있는데, 여기서 중복으로 장르 매핑 로직을 구현하고 있습니다.GenreManager의 메서드를 재사용하는 것이 좋겠습니다.다음과 같이 수정하세요:
private fun getApiCategoryName(genreIndex: Int): String { val currentGenres = uiState.value.genres if (genreIndex >= 0 && genreIndex < currentGenres.size) { val genre = currentGenres[genreIndex] - return when (genre) { - "과학·IT" -> "과학/IT" - else -> genre - } + return genreManager.mapGenreToApiCategory(genre) } - return "문학" + return genreManager.mapGenreToApiCategory(genreManager.getDefaultGenre()) }app/src/main/java/com/texthip/thip/data/repository/GroupRepository.kt (1)
135-141: type 파라미터의 타입 안전성 개선이 필요합니다.
type파라미터가String으로 되어 있어 잘못된 값이 전달될 수 있습니다. enum이나 sealed class 사용을 고려해보세요.enum class RoomActionType(val value: String) { JOIN("join"), CANCEL("cancel") } suspend fun joinOrCancelRoom(roomId: Int, actionType: RoomActionType): Result<String> { return runCatching { val request = RoomJoinRequest(type = actionType.value) groupService.joinOrCancelRoom(roomId, request) .handleBaseResponse() .getOrThrow() ?.type ?: throw Exception("Failed to join/cancel room: no response") } }
🧹 Nitpick comments (3)
app/src/main/java/com/texthip/thip/ui/group/done/screen/GroupDoneScreen.kt (1)
94-103: CardItemRoom 사용법이 적절하지만 주석을 개선해보세요.카드 아이템 구현이 잘 되어있습니다:
imageUrl사용으로 새로운 이미지 처리 방식 적용- 적절한 데이터 매핑 (
recruitCount를maxParticipants로 사용)- 확장 함수
isRecruitingByType()활용다만 주석을 좀 더 명확하게 할 수 있습니다:
- onClick = { /* 완료된 모임방은 클릭 불가 */ } + onClick = { /* 완료된 모임방은 상세보기 불가 */ }app/src/main/java/com/texthip/thip/ui/group/myroom/viewmodel/GroupMyViewModel.kt (1)
22-24: 페이지네이션 상태 변수들의 접근성 개선 필요
nextCursor,isLastPage,isLoadingData변수들이 private이지만 클래스 레벨에 정의되어 있습니다. 이들을UiState에 포함시키거나 더 명확한 상태 관리 패턴을 고려해보세요.// UiState에 포함하는 방법: data class GroupMyUiState( // ... 기존 프로퍼티들 val nextCursor: String? = null, val isLastPage: Boolean = false, val isLoadingData: Boolean = false )app/src/main/java/com/texthip/thip/ui/group/myroom/screen/GroupMyScreen.kt (1)
96-122: 필터 토글 로직의 복잡성현재 필터 토글 로직이 복잡하고 이해하기 어렵습니다. 각 버튼을 클릭했을 때의 상태 전환이 직관적이지 않을 수 있습니다.
다음과 같이 더 명확한 로직으로 리팩터링을 고려해보세요:
onToggle = { idx -> val currentType = uiState.currentRoomType val newRoomType = when (idx) { 0 -> when (currentType) { RoomType.PLAYING -> RoomType.PLAYING_AND_RECRUITING else -> RoomType.PLAYING } 1 -> when (currentType) { RoomType.RECRUITING -> RoomType.PLAYING_AND_RECRUITING else -> RoomType.RECRUITING } else -> RoomType.PLAYING_AND_RECRUITING } viewModel.changeRoomType(newRoomType) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
app/src/main/res/drawable/img_book_cover_sample.pngis excluded by!**/*.png
📒 Files selected for processing (37)
app/src/main/java/com/texthip/thip/data/manager/GenreManager.kt(1 hunks)app/src/main/java/com/texthip/thip/data/manager/UserDataManager.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/book/response/BookListResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/repository/BookRepository.kt(1 hunks)app/src/main/java/com/texthip/thip/data/repository/GroupRepository.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/common/buttons/GenreChipRow.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/common/cards/CardBookList.kt(3 hunks)app/src/main/java/com/texthip/thip/ui/common/cards/CardBookSearch.kt(3 hunks)app/src/main/java/com/texthip/thip/ui/common/cards/CardInputBook.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/common/cards/CardItemRoom.kt(5 hunks)app/src/main/java/com/texthip/thip/ui/common/cards/CardRoomBook.kt(3 hunks)app/src/main/java/com/texthip/thip/ui/feed/component/MyFeedCard.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/feed/screen/FeedCommentScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/done/mock/MyRoomCardData.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/done/screen/GroupDoneScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/done/viewmodel/GroupDoneUiState.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/done/viewmodel/GroupDoneViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupSelectBook.kt(3 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomUiState.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/myroom/component/GroupMainCard.kt(3 hunks)app/src/main/java/com/texthip/thip/ui/group/myroom/mock/RoomType.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/myroom/screen/GroupMyScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/myroom/viewmodel/GroupMyUiState.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/myroom/viewmodel/GroupMyViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/room/mock/RoomAction.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/room/viewmodel/GroupRoomRecruitUiState.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/room/viewmodel/GroupRoomRecruitViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupUiState.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/mypage/component/SavedFeedCard.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/mypage/viewmodel/SavedFeedViewModel.kt(3 hunks)app/src/main/java/com/texthip/thip/ui/navigator/navigations/GroupNavigation.kt(6 hunks)app/src/main/java/com/texthip/thip/ui/search/mock/DetailBookData.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookDetailScreen.kt(1 hunks)app/src/main/res/values/strings.xml(1 hunks)
✅ Files skipped from review due to trivial changes (12)
- app/src/main/java/com/texthip/thip/ui/feed/component/MyFeedCard.kt
- app/src/main/java/com/texthip/thip/ui/mypage/component/SavedFeedCard.kt
- app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt
- app/src/main/java/com/texthip/thip/ui/feed/screen/FeedCommentScreen.kt
- app/src/main/java/com/texthip/thip/ui/mypage/viewmodel/SavedFeedViewModel.kt
- app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookDetailScreen.kt
- app/src/main/java/com/texthip/thip/ui/common/cards/CardInputBook.kt
- app/src/main/java/com/texthip/thip/ui/search/mock/DetailBookData.kt
- app/src/main/java/com/texthip/thip/ui/group/myroom/mock/RoomType.kt
- app/src/main/java/com/texthip/thip/ui/common/cards/CardRoomBook.kt
- app/src/main/java/com/texthip/thip/ui/group/done/viewmodel/GroupDoneUiState.kt
- app/src/main/java/com/texthip/thip/ui/group/room/viewmodel/GroupRoomRecruitUiState.kt
🚧 Files skipped from review as they are similar to previous changes (13)
- app/src/main/java/com/texthip/thip/ui/common/buttons/GenreChipRow.kt
- app/src/main/java/com/texthip/thip/ui/group/myroom/component/GroupMainCard.kt
- app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupSelectBook.kt
- app/src/main/java/com/texthip/thip/ui/common/cards/CardItemRoom.kt
- app/src/main/java/com/texthip/thip/data/repository/BookRepository.kt
- app/src/main/java/com/texthip/thip/ui/common/cards/CardBookList.kt
- app/src/main/java/com/texthip/thip/data/manager/UserDataManager.kt
- app/src/main/java/com/texthip/thip/data/manager/GenreManager.kt
- app/src/main/java/com/texthip/thip/ui/common/cards/CardBookSearch.kt
- app/src/main/java/com/texthip/thip/ui/group/done/viewmodel/GroupDoneViewModel.kt
- app/src/main/java/com/texthip/thip/ui/group/done/mock/MyRoomCardData.kt
- app/src/main/res/values/strings.xml
- app/src/main/java/com/texthip/thip/ui/group/room/viewmodel/GroupRoomRecruitViewModel.kt
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: rbqks529
PR: THIP-TextHip/THIP-Android#30
File: app/src/main/java/com/texthip/thip/ui/common/cards/CardItemRoomSmall.kt:108-112
Timestamp: 2025-07-01T07:19:10.981Z
Learning: The user rbqks529 prefers to extract hardcoded UI strings to StringResource files (strings.xml) rather than keeping them inline in Compose components, which is a good practice for maintainability and localization in Android development.
📚 Learning: grouppagescreen의 mypageviewmodel은 임시 viewmodel로, 현재는 프로토타입 단계이므로 오류 처리 등의 추가 기능은 나중에 구현 예정입니다....
Learnt from: rbqks529
PR: THIP-TextHip/THIP-Android#30
File: app/src/main/java/com/texthip/thip/ui/myPage/groupPage/GroupPageScreen.kt:34-37
Timestamp: 2025-07-01T07:19:01.239Z
Learning: GroupPageScreen의 MyPageViewModel은 임시 ViewModel로, 현재는 프로토타입 단계이므로 오류 처리 등의 추가 기능은 나중에 구현 예정입니다.
Applied to files:
app/src/main/java/com/texthip/thip/ui/group/myroom/viewmodel/GroupMyViewModel.ktapp/src/main/java/com/texthip/thip/ui/group/done/screen/GroupDoneScreen.ktapp/src/main/java/com/texthip/thip/ui/group/myroom/screen/GroupMyScreen.ktapp/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.ktapp/src/main/java/com/texthip/thip/ui/navigator/navigations/GroupNavigation.ktapp/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt
📚 Learning: the user rbqks529 prefers to extract hardcoded ui strings to stringresource files (strings.xml) rath...
Learnt from: rbqks529
PR: THIP-TextHip/THIP-Android#30
File: app/src/main/java/com/texthip/thip/ui/common/cards/CardItemRoomSmall.kt:108-112
Timestamp: 2025-07-01T07:19:10.981Z
Learning: The user rbqks529 prefers to extract hardcoded UI strings to StringResource files (strings.xml) rather than keeping them inline in Compose components, which is a good practice for maintainability and localization in Android development.
Applied to files:
app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt
🧬 Code Graph Analysis (2)
app/src/main/java/com/texthip/thip/ui/group/myroom/viewmodel/GroupMyViewModel.kt (4)
app/src/main/java/com/texthip/thip/ui/group/done/viewmodel/GroupDoneViewModel.kt (1)
updateState(27-29)app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt (1)
updateState(36-38)app/src/main/java/com/texthip/thip/ui/group/room/viewmodel/GroupRoomRecruitViewModel.kt (1)
updateState(29-31)app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt (1)
updateState(33-35)
app/src/main/java/com/texthip/thip/ui/navigator/navigations/GroupNavigation.kt (3)
app/src/main/java/com/texthip/thip/ui/group/myroom/screen/GroupMyScreen.kt (1)
GroupMyScreen(40-169)app/src/main/java/com/texthip/thip/ui/group/room/screen/GroupRoomRecruitScreen.kt (1)
GroupRoomRecruitScreen(54-441)app/src/main/java/com/texthip/thip/ui/group/room/screen/GroupRoomScreen.kt (1)
GroupRoomScreen(40-179)
🪛 detekt (1.23.8)
app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt
[warning] 81-81: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🔇 Additional comments (35)
app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomUiState.kt (2)
3-4: 올바른 임포트 구조입니다.mock 패키지에서
BookData와GroupMakeRoomRequest를 임포트하는 것이 적절합니다. API 연동 과정에서 점진적으로 실제 데이터 모델로 전환하는 단계로 보입니다.
20-24: 새로운 UI 상태 속성들이 잘 설계되었습니다.API 연동을 위한 새로운 속성들이 적절히 추가되었습니다:
savedBooks와groupBooks: 각각 저장된 책과 그룹 책 목록을 관리isLoadingBooks: 책 데이터 로딩 상태를 별도로 관리genres: 장르 목록을 동적으로 관리- 모든 속성이 적절한 기본값을 가지고 있어 안전합니다
trailing comma 추가로 향후 속성 추가 시 diff가 깔끔해집니다.
app/src/main/java/com/texthip/thip/data/model/book/response/BookListResponse.kt (1)
7-19: 잘 설계된 API 응답 모델입니다.API 응답을 위한 데이터 모델이 우수하게 구현되었습니다:
@Serializable과@SerialName어노테이션이 적절히 사용됨imageUrl이 nullable로 처리되어 실제 API 응답의 다양성을 반영- 명확한 프로퍼티 이름과 적절한 타입 사용
- 중첩된 응답 구조가 잘 모델링됨
API 연동을 위한 견고한 기반이 될 것입니다.
app/src/main/java/com/texthip/thip/ui/group/myroom/viewmodel/GroupMyUiState.kt (1)
6-16: 훌륭한 UI 상태 관리 설계입니다.
GroupMyUiState클래스가 매우 잘 구성되었습니다:
- 페이지네이션 지원을 위한 완전한 상태 관리 (
isLoadingMore,hasMore)- 계산된 프로퍼티
hasContent와canLoadMore가 UI 로직을 단순화- 적절한 기본값과 타입 안전성 보장
- 무한 스크롤과 pull-to-refresh 패턴에 최적화된 구조
특히
canLoadMore로직이 여러 로딩 상태를 종합적으로 고려하여 안전하게 구현되었습니다.app/src/main/java/com/texthip/thip/ui/group/done/screen/GroupDoneScreen.kt (4)
35-57: 무한 스크롤 구현이 효율적이고 안전합니다.무한 스크롤 로직이 잘 구현되었습니다:
derivedStateOf를 사용한 효율적인 상태 추적- 마지막 3개 아이템에서 트리거하는 적절한 임계값
canLoadMore조건 검사로 중복 로딩 방지LaunchedEffect를 통한 side effect 관리성능과 사용자 경험 모두를 고려한 구현입니다.
67-71: PullToRefreshBox 사용이 적절합니다.Material3의
PullToRefreshBox를 사용한 새로고침 기능이 잘 구현되었습니다:
uiState.isLoading을 통한 적절한 로딩 상태 연결- ViewModel의
refreshData()메서드와 올바른 연동사용자 친화적인 새로고침 경험을 제공합니다.
86-92: 문자열 리소스 사용이 훌륭합니다.문자열 리소스를 적절히 활용하여 현지화와 유지보수성을 고려한 구현입니다. 사용자의 선호도에 부합하는 좋은 관행입니다.
112-119: Preview 구현이 적절합니다.ViewModel을 사용할 수 없는 Preview 환경에 대한 적절한 처리와 주석이 포함되어 있어 좋습니다. 개발 시 레이아웃 확인에 유용할 것입니다.
app/src/main/java/com/texthip/thip/ui/group/myroom/viewmodel/GroupMyViewModel.kt (3)
34-36: 동시 요청 방지 로직의 일관성 확인
reset파라미터에 따른 조건문 처리가 있지만,isLoadingData체크와isLastPage체크의 순서와 로직이 명확합니다. 하지만reset=true일 때 이 체크들을 우회하는 것이 의도된 동작인지 확인이 필요합니다.
50-61: API 응답 처리 로직 검증
paginationResult.data를 현재 리스트에 추가하는 로직과hasMore필드를!paginationResult.isLast로 매핑하는 부분이 올바릅니다. 커서 기반 페이지네이션이 적절히 구현되었습니다.
81-86: 필터 변경 시 중복 호출 방지현재
roomType이 다를 때만 상태를 업데이트하고 데이터를 다시 로드하는 로직이 올바르게 구현되어 있습니다. 불필요한 API 호출을 방지합니다.app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupUiState.kt (3)
18-20: 계산된 프로퍼티 로직 검증
hasContent와canLoadMore프로퍼티의 로직이 올바릅니다. 특히canLoadMore에서 여러 조건을 적절히 체크하여 무한 스크롤 동작을 제어합니다.
6-16: UI 상태 클래스 구조가 잘 설계됨로딩 상태, 페이지네이션, 에러 처리, 토스트 알림 등 그룹 기능에 필요한 모든 UI 상태를 체계적으로 관리하고 있습니다. 단일 상태 객체로 통합하여 상태 관리가 단순화되었습니다.
3-4: Mock 데이터 모델 광범위한 사용 현황 및 마이그레이션 계획 필요
현재GroupCardData,GroupRoomSectionData를 포함한 다수의 mock 데이터 클래스가 리포지토리, 매퍼, UI 뷰모델 및 컴포넌트 전반에 걸쳐 사용되고 있습니다. 실제 API 연동 이후에는 아래 파일들을 비롯한 관련 코드를 실제 도메인 모델로 교체해야 합니다:
- app/src/main/java/com/texthip/thip/data/repository/GroupRepository.kt
- app/src/main/java/com/texthip/thip/data/mapper/GroupDataMapper.kt
- app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupUiState.kt
- app/src/main/java/com/texthip/thip/ui/group/myroom/component/GroupDeadlineRoomSection.kt
- 그 외 다수의 UI 컴포넌트 및 화면
실제 모델 정의, 매핑 구조 설계, 그리고 교체 작업 시 데이터 흐름(리포지토리 → 매퍼 → UI)이 올바르게 연결되는지 사전에 검토하고 계획을 수립해주세요.
app/src/main/java/com/texthip/thip/ui/group/myroom/screen/GroupMyScreen.kt (3)
51-63: 무한 스크롤 구현이 올바름
derivedStateOf를 사용하여 스크롤 위치를 모니터링하고,LaunchedEffect로 적절한 조건에서 추가 데이터를 로드하는 로직이 잘 구현되었습니다. 3개 아이템 전에 미리 로드하는 것도 좋은 UX입니다.
134-143: 확장 함수 사용이 적절함
room.isRecruitingByType()과room.getEndDateInDays()확장 함수를 사용하여 비즈니스 로직을 캡슐화한 것이 좋습니다. 코드 가독성과 재사용성이 향상되었습니다.
145-165: 빈 상태 처리가 적절함로딩 상태를 확인하여 빈 상태 메시지를 표시하는 로직이 올바릅니다. 사용자에게 명확한 피드백을 제공합니다.
app/src/main/java/com/texthip/thip/ui/navigator/navigations/GroupNavigation.kt (6)
44-52: 토스트 메시지 전달 메커니즘이 잘 구현됨
savedStateHandle을 사용하여 화면 간 토스트 메시지를 전달하는 패턴이 깔끔합니다. 메시지 전달 후 즉시 제거하여 중복 표시를 방지하는 것도 좋습니다.
105-109: 일관된 ViewModel 주입 패턴모든 화면에서
hiltViewModel()을 사용하여 일관된 의존성 주입 패턴을 유지하고 있습니다. 이전의 수동 ViewModel 관리에서 Hilt로 전환한 것이 좋은 개선입니다.
125-129: roomSections 데이터 평탄화 로직검색 화면에서
uiState.roomSections.flatMap { it.rooms }를 사용하여 룸 리스트를 평탄화하는 것이 적절합니다. 주석으로 "임시"라고 표시되어 있어 향후 개선 계획이 있음을 알 수 있습니다.
148-162: GroupRoomRecruitScreen 파라미터 단순화이전의 복잡한 콜백들을 제거하고
roomId와 간단한 네비게이션 콜백만 전달하는 것이 좋은 개선입니다. ViewModel에서 상태 관리를 담당하도록 책임을 분리했습니다.
167-171: GroupRoomScreen 단순화GroupRoomScreen에서 불필요한 ViewModel 주입과 복잡한 로직을 제거한 것이 좋습니다. 단순한 뒤로가기 콜백만 남겨두어 화면의 책임을 명확히 했습니다.
110-115:roomId필드 일관적 사용 확인 – 추가 수정 불필요코드베이스 전반에서
roomId속성이 일관되게 사용되고 있으며,
- GroupRepository.kt (방 생성 반환값 처리)
- GroupDataMapper.kt (DTO → 도메인 매핑)
- MyRoomCardData.kt (UI용 데이터 클래스)
- GroupNavigation.kt (네비게이션 경로)
모두
roomId로 정의되어 API 스펙과 상충하지 않습니다.
따라서 이전id필드 관련 추가 검증이나 수정은 필요하지 않습니다.app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt (5)
24-25: 단일 상태 객체 도입이 우수함여러 개별 StateFlow를 하나의
GroupUiState로 통합한 것이 훌륭한 개선입니다. 상태 관리가 단순화되고 UI와의 동기화가 더 안정적이 되었습니다.
68-100: 배치 로딩 로직이 잘 구현됨페이지를 배치 단위로 로드하는 로직이 효율적입니다.
isBatchLoading플래그로 중복 요청을 방지하고, 실패 시 적절히 중단하는 로직도 좋습니다.
120-141: 장르 기반 룸 섹션 로딩의 안전한 처리장르 목록을 먼저 가져온 후 안전하게 선택된 장르를 결정하고, 실패 시 기본값으로 대체하는 로직이 견고합니다. 에러 상태도 적절히 업데이트합니다.
156-174: 동시성 데이터 새로고침
async와awaitAll()을 사용하여 여러 데이터를 동시에 로드하는 것이 효율적입니다. 새로고침 상태 관리도 적절히 구현되었습니다.
187-198: 토스트 메시지 관리 로직토스트 표시와 숨기기 메서드가 간단하고 명확합니다. UI에서 호출하기 쉬운 인터페이스를 제공합니다.
app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt (2)
3-27: 의존성 주입 구조가 잘 설계되었습니다.Hilt를 사용한 의존성 주입으로 테스트 가능성과 유지보수성이 크게 향상되었습니다. Context 주입을 통해 문자열 리소스에 접근할 수 있게 된 점도 좋습니다.
32-51: 상태 관리 중앙화가 잘 구현되었습니다.
updateState함수를 통한 상태 업데이트 중앙화와init블록에서의 비동기 장르 로딩이 적절합니다. 코드 일관성과 유지보수성이 향상되었습니다.app/src/main/java/com/texthip/thip/data/repository/GroupRepository.kt (5)
19-26: 의존성 주입 설계가 잘 구성되었습니다.Singleton 패턴과 Hilt 의존성 주입을 통해 서비스, 매퍼, 매니저들이 적절히 주입되고 있습니다. 관심사의 분리가 잘 되어 있습니다.
29-36: 캐시된 데이터 접근 방식이 효율적입니다.
GenreManager와UserDataManager를 통해 캐시된 데이터에 접근하는 방식이 효율적입니다.Result타입 래핑도 일관성 있게 처리되었습니다.
39-62: 안전한 데이터 처리 로직이 잘 구현되었습니다.
runCatching과handleBaseResponse()를 통한 예외 처리, null 안전성 처리, 그리고 실패 시 기본값 반환 로직이 견고하게 구현되었습니다.
65-92: 데이터 매핑과 섹션 구성 로직이 체계적입니다.API 카테고리 매핑, 문자열 리소스 활용, 그리고 마감임박/인기 섹션으로 구분한 데이터 구조가 UI 요구사항을 잘 반영하고 있습니다.
127-132: 모임방 생성 로직이 안전하게 구현되었습니다.
runCatching을 통한 예외 처리와 null 체크를 통한 안전한 데이터 반환이 잘 구현되었습니다.
| package com.texthip.thip.ui.group.room.mock | ||
|
|
||
| enum class RoomAction(val value: String) { | ||
| JOIN("join"), | ||
| CANCEL("cancel") | ||
| } No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
enum 클래스 구조는 좋으나 패키지 위치를 재고해보세요.
RoomAction enum의 구조와 값 매핑은 적절하지만, 실제 API 연동에 사용되는 코드가 mock 패키지에 위치하는 것은 부적절합니다.
다음과 같이 리팩토링을 권장합니다:
-package com.texthip.thip.ui.group.room.mock
+package com.texthip.thip.ui.group.room.model또는 더 범용적인 위치로:
-package com.texthip.thip.ui.group.room.mock
+package com.texthip.thip.data.model.group🤖 Prompt for AI Agents
In app/src/main/java/com/texthip/thip/ui/group/room/mock/RoomAction.kt lines 1
to 6, the RoomAction enum is correctly structured but is misplaced in the mock
package. Move this enum out of the mock package to a more appropriate location
such as a core or common package related to API models or domain logic, for
example, com.texthip.thip.ui.group.room or com.texthip.thip.model, to better
reflect its actual usage in API integration.
JJUYAAA
left a comment
JJUYAAA
left a comment
There was a problem hiding this comment.
코드 확인 완료햇숩니다~ files changed 78!!!!🫨🫨🫨ㅋㅋㅋㅎㅋㅋ
궁금한 점이 있어 리뷰남겼는데 답변부탁드립니당
| suspend fun getMyRoomsByType(type: String?, cursor: String? = null) = runCatching { | ||
| groupService.getMyRooms(type, cursor) | ||
| .handleBaseResponse() | ||
| .getOrThrow() | ||
| ?.let { data -> | ||
| val myRoomCards = data.roomList.map { room -> | ||
| groupDataMapper.toMyRoomCardData(room) | ||
| } | ||
|
|
||
| MyRoomsPaginationResult( | ||
| data = myRoomCards, | ||
| nextCursor = data.nextCursor, | ||
| isLast = data.isLast | ||
| ) | ||
| } ?: MyRoomsPaginationResult( | ||
| data = emptyList(), | ||
| nextCursor = null, | ||
| isLast = true | ||
| ) | ||
| } |
There was a problem hiding this comment.
오 저는 runCatching을 처음 봐서 좀 찾아봤습니다. 약간의 의문이 생겨서 질문남깁니다.
?: 구문은 앞 ( .let{~~} ) 구문의 결과가 null일 때 실행됩니다.
근데 getOtThrow()를 해버리면
성공시: .let 실행 ( 이 경우는 Result.Success인 경우 -> MyRoomPaginationResult 반환 -> null아님) ,
실패 시: 예외를 던져서 runCatching이 잡아서 Result.Failure로 감싸서 ?: 까지 도달하지 못할 것 같은데, 이부분은 구현하신 대로 예외처리가 되는지 테스트 해보셨나용??
즉, ?:는 성공인데 null인 경우 실행될 것 같은데, 그런 경우가 현재구조로서는 없지 않나 싶습니당
There was a problem hiding this comment.
음 듣고 보니까 그렇네요 런캐칭으로 바꾸는 과정에서 모순이 있었던거 같네요 수정하겠습니다
|
|
||
| fun mapGenreToApiCategory(genre: String): String { | ||
| return when (genre) { | ||
| context.getString(R.string.science_it) -> context.getString(R.string.api_genre_science_it) | ||
| else -> genre | ||
| } | ||
| } |
| isRecruiting = true, | ||
| endDate = extractDaysFromDeadline(dto.recruitEndDate), | ||
| imageUrl = dto.roomImageUrl, | ||
| genreIndex = 0, |
| fun cacheUserName(name: String) { | ||
| cachedUserName = name | ||
| } |
There was a problem hiding this comment.
오옹 캐싱 처리도 해주셨네용 👍👍
앱을 종료하기 전까지만 유저네임을 보존하고 앱을 종료하면 휘발되게끔 하려는 목적이 맞으신가요? 전 좋은 것 같습니당
| private val genres = listOf( | ||
| context.getString(R.string.literature), | ||
| context.getString(R.string.science_it), | ||
| context.getString(R.string.social_science), | ||
| context.getString(R.string.humanities), | ||
| context.getString(R.string.art) | ||
| ) |
There was a problem hiding this comment.
https://developer.android.com/topic/architecture?hl=ko
구글의 앱 아키텍처 or 클린 아키텍처를 봤을 때, data 레이어에서는 ui 레이어에 대한 의존성이 존재하지 않아야 합니다.
불가피한 경우에 Context 같은 애는 의존성 주입으로 넣어서 유동적으로 구현해도 되겠지만,
결론적으로 이 경우에는 Genre 에 대한 enum 모델을 만들어 구현하는 게 어떤가 싶어요!
만약 장르에 관한 stringResource 가 변경될 때 or data 레이어에서 사용할 장르가 변경될 때(ex) api 수정, entity 변경 등등..), data 레이어와 ui 레이어 모두 수정이 필요할 수 있습니다.
데이터 레이어에서는 genres 를 Genre.entries 로 사용하고, 스크린 단에서
when(genre) {
Genre.SOME_GENRE -> TODO()
}로 하는게 어떠한가 추천드립니다
| ?.let { roomsData -> | ||
| listOf( | ||
| GroupRoomSectionData( | ||
| title = context.getString(R.string.room_section_deadline), | ||
| rooms = roomsData.deadlineRoomList.map { dto -> | ||
| val daysLeft = groupDataMapper.extractDaysFromDeadline(dto.deadlineDate) | ||
| groupDataMapper.toGroupCardItemRoomData(dto, daysLeft) | ||
| }, | ||
| genres = genreManager.getGenres() | ||
| ), | ||
| GroupRoomSectionData( | ||
| title = context.getString(R.string.room_section_popular), | ||
| rooms = roomsData.popularRoomList.map { dto -> | ||
| val daysLeft = groupDataMapper.extractDaysFromDeadline(dto.deadlineDate) | ||
| groupDataMapper.toGroupCardItemRoomData(dto, daysLeft) | ||
| }, | ||
| genres = genreManager.getGenres() | ||
| ) | ||
| ) | ||
| }.orEmpty() | ||
| } |
There was a problem hiding this comment.
Mapper 함수를 통해 깔끔하게 처리하면 어떤가 싶습니다!!
toUiState() 같은 확장함수로 구현한다면, repository 클래스 코드도 깔끔해지고, 수정에 조금 더 유연하게 대처할 수 있을 거 같아요!
There was a problem hiding this comment.
Actionable comments posted: 11
🔭 Outside diff range comments (1)
app/src/main/java/com/texthip/thip/ui/group/search/screen/GroupSearchScreen.kt (1)
118-122: 마감임박 정렬 시 endDate=null 항목이 최상단으로 오는 문제(nulls last 권장)
sortedBy { it.endDate }는 null을 먼저 배치합니다. UX상 마감일 미정(null)은 마지막으로 보내는 것이 자연스럽습니다.- when (selectedSortOptionIndex) { - 0 -> filtered.sortedBy { it.endDate } // 마감임박순 + when (selectedSortOptionIndex) { + 0 -> filtered + .sortedWith( + compareBy<GroupCardItemRoomData> { it.endDate == null } + .thenBy { it.endDate } + ) // 마감임박순 (null은 가장 뒤) 1 -> filtered.sortedByDescending { it.participants } // 인기순 else -> filtered }
♻️ Duplicate comments (5)
app/src/main/java/com/texthip/thip/data/repository/GroupRepository.kt (1)
79-86: 문자열 파라미터 대신 enum 사용 권장
이미 이전 리뷰에서 동일 지적이 있었습니다. 타입 안정성을 위해RoomActionTypeenum으로 교체해 주세요.app/src/main/java/com/texthip/thip/ui/group/screen/GroupScreen.kt (1)
53-56: 화면 재진입 새로고침 로직 개선 필요
LaunchedEffect(Unit)은 최초 컴포지션에서만 실행됩니다. 화면 재포커스 시 API 재호출이 필요하다면LifecycleEventObserver또는currentBackStackEntry를 key 로 사용하도록 수정해 주세요.app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt (1)
108-116: 매직 넘버 3 상수화 필요
카드당 페이지 계산에 하드코딩된3이 그대로 남아 있습니다.CARDS_PER_PAGE상수로 추출해 사용해 주세요.app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt (2)
181-188: 장르 매핑 로직은 GenreManager 재사용 권장getApiCategoryName에서 인덱스→카테고리 문자열 매핑을 직접 처리하고 있습니다. 과거 리뷰에서도 지적되었듯, GenreManager의 매핑 메서드 재사용 또는 Repository에 위임하는 편이 중복 제거와 변경 내성에 유리합니다.
- 옵션 A: ViewModel에 GenreManager 주입 후 mapGenreToApiCategory(또는 동등 기능) 호출
- 옵션 B: createRoom 호출 시 index를 넘기고 Repository(내부 GenreManager)가 네트워크 카테고리로 변환
82-84: 예외를 삼키고 있습니다 — 로깅 추가 필요detekt가 지적한 그대로 catch 블록에서 예외를 삼키고 있습니다. 최소한의 로깅을 추가하세요.
} catch (e: Exception) { - updateState { it.copy(savedBooks = emptyList(), groupBooks = emptyList()) } + // TODO: 로깅 프레임워크 적용 시 교체 + android.util.Log.e("GroupMakeRoomViewModel", "Failed to load books", e) + updateState { it.copy(savedBooks = emptyList(), groupBooks = emptyList()) } }
🧹 Nitpick comments (8)
app/src/main/java/com/texthip/thip/ui/group/search/screen/GroupSearchScreen.kt (1)
244-273: 프리뷰에서도 URL 기반 이미지를 사용해 실제 렌더링 경로 검증 권장프리뷰의
imageUrl = null은 URL 로더(예: Coil) 경로를 검증하기 어렵습니다. 샘플 URL을 넣어 카드 레이아웃/로딩 플레이스홀더를 확인하는 것을 권장합니다.- imageUrl = null, + imageUrl = "https://picsum.photos/300/400?seed=1", ... - imageUrl = null, + imageUrl = "https://picsum.photos/300/400?seed=2", ... - imageUrl = null, + imageUrl = "https://picsum.photos/300/400?seed=3",app/src/main/java/com/texthip/thip/data/manager/Genre.kt (1)
17-23: API 응답 역매핑 유틸 추가 제안(fromApiCategory / fromNetworkApiCategory)서버 응답 문자열을 enum으로 역매핑하는 유틸이 있으면 Repository 단 코드가 단순해집니다.
companion object { fun getDefault() = LITERATURE fun fromDisplayKey(displayKey: String): Genre? { return entries.find { it.displayKey == displayKey } } + + fun fromApiCategory(category: String): Genre? { + return entries.find { it.apiCategory == category } + } + + fun fromNetworkApiCategory(category: String): Genre? { + return entries.find { it.networkApiCategory == category } + } }app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt (1)
3-22: 주입된Context미사용
@ApplicationContext Context를 주입했지만 본 클래스 내부에서 사용하지 않습니다. 불필요한 의존성은 제거하거나 실제 용도를 추가해 주세요.app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt (2)
58-63: 시트 열 때마다 매번 네트워크 호출됨 – 캐시 고려사용자 경험과 네트워크 비용을 위해 최초 로드/변경 시에만 불러오고, 이미 로드된 경우 캐시된 리스트를 사용하는 방식을 고려하세요.
69-69: "saved"/"joining" 매직 스트링 제거타입 안전성과 오탈자 방지를 위해 enum/sealed class 또는 상수로 치환하세요. 예: BookListType.SAVED/JOINING 또는 Companion 상수.
Also applies to: 76-76
app/src/main/java/com/texthip/thip/ui/group/room/screen/GroupRoomRecruitScreen.kt (3)
70-75: LaunchedEffect 키 단순화로 중복 트리거 방지키를 두 개(shouldNavigateToGroupScreen, toastMessage)로 두면 메시지 변경에도 네비게이션이 재평가될 수 있습니다. 네비게이션 플래그만 키로 두는 것이 안전합니다.
-LaunchedEffect(uiState.shouldNavigateToGroupScreen, uiState.toastMessage) { +LaunchedEffect(uiState.shouldNavigateToGroupScreen) { if (uiState.shouldNavigateToGroupScreen) { onNavigateToGroupScreen(uiState.toastMessage) viewModel.onNavigatedToGroupScreen() } }
119-126: contentDescription 등 하드코딩된 접근성 문자열을 리소스로 추출팀 선호도(과거 PR 학습)와 현지화/접근성 모범사례에 맞춰 strings.xml로 이동하세요.
제안 키 예:
- background_image_desc
- room_private_desc
- room_public_desc
- period_icon_desc
- participants_icon_desc
적용 예:
- contentDescription = "배경 이미지" + contentDescription = stringResource(R.string.background_image_desc)Also applies to: 172-179, 180-186, 214-219, 246-251
302-308: D-n 파싱은 ViewModel에서 계산해 전달 권장Compose에서 매번 daysLeft를 계산하기보다 UI 상태로 값을 내려주면 재조합 비용을 줄이고 테스트 용이성이 향상됩니다.
Also applies to: 362-370
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
app/src/main/java/com/texthip/thip/data/manager/Genre.kt(1 hunks)app/src/main/java/com/texthip/thip/data/manager/GenreManager.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/group/response/JoinedRoomListResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/group/response/MyRoomListResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/repository/GroupRepository.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/done/screen/GroupDoneScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/done/viewmodel/GroupDoneUiState.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/done/viewmodel/GroupDoneViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt(11 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomUiState.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/myroom/component/GroupDeadlineRoomSection.kt(5 hunks)app/src/main/java/com/texthip/thip/ui/group/myroom/component/GroupMainCard.kt(7 hunks)app/src/main/java/com/texthip/thip/ui/group/myroom/component/GroupPager.kt(5 hunks)app/src/main/java/com/texthip/thip/ui/group/myroom/mock/GroupCardItemRoomData.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/myroom/screen/GroupMyScreen.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/group/myroom/viewmodel/GroupMyUiState.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/myroom/viewmodel/GroupMyViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/room/screen/GroupRoomRecruitScreen.kt(14 hunks)app/src/main/java/com/texthip/thip/ui/group/room/viewmodel/GroupRoomRecruitUiState.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/room/viewmodel/GroupRoomRecruitViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/screen/GroupScreen.kt(5 hunks)app/src/main/java/com/texthip/thip/ui/group/search/component/GroupFilteredSearchResult.kt(3 hunks)app/src/main/java/com/texthip/thip/ui/group/search/component/GroupLiveSearchResult.kt(4 hunks)app/src/main/java/com/texthip/thip/ui/group/search/screen/GroupSearchScreen.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupUiState.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/navigator/navigations/GroupNavigation.kt(6 hunks)app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookGroupScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/utils/rooms/DateUtils.kt(1 hunks)app/src/main/java/com/texthip/thip/utils/rooms/GenreExtensions.kt(1 hunks)app/src/main/java/com/texthip/thip/utils/rooms/RoomUtils.kt(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookGroupScreen.kt
- app/src/main/java/com/texthip/thip/utils/rooms/GenreExtensions.kt
- app/src/main/java/com/texthip/thip/utils/rooms/DateUtils.kt
🚧 Files skipped from review as they are similar to previous changes (17)
- app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomUiState.kt
- app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupUiState.kt
- app/src/main/java/com/texthip/thip/ui/group/search/component/GroupFilteredSearchResult.kt
- app/src/main/java/com/texthip/thip/data/model/group/response/JoinedRoomListResponse.kt
- app/src/main/java/com/texthip/thip/ui/group/myroom/component/GroupMainCard.kt
- app/src/main/java/com/texthip/thip/data/model/group/response/MyRoomListResponse.kt
- app/src/main/java/com/texthip/thip/ui/group/done/viewmodel/GroupDoneViewModel.kt
- app/src/main/java/com/texthip/thip/ui/group/myroom/viewmodel/GroupMyUiState.kt
- app/src/main/java/com/texthip/thip/data/manager/GenreManager.kt
- app/src/main/java/com/texthip/thip/ui/group/room/viewmodel/GroupRoomRecruitUiState.kt
- app/src/main/java/com/texthip/thip/ui/group/done/screen/GroupDoneScreen.kt
- app/src/main/java/com/texthip/thip/ui/group/done/viewmodel/GroupDoneUiState.kt
- app/src/main/java/com/texthip/thip/ui/group/myroom/component/GroupPager.kt
- app/src/main/java/com/texthip/thip/ui/group/myroom/component/GroupDeadlineRoomSection.kt
- app/src/main/java/com/texthip/thip/ui/group/myroom/viewmodel/GroupMyViewModel.kt
- app/src/main/java/com/texthip/thip/ui/group/room/viewmodel/GroupRoomRecruitViewModel.kt
- app/src/main/java/com/texthip/thip/ui/group/search/component/GroupLiveSearchResult.kt
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: rbqks529
PR: THIP-TextHip/THIP-Android#30
File: app/src/main/java/com/texthip/thip/ui/common/cards/CardItemRoomSmall.kt:108-112
Timestamp: 2025-07-01T07:19:10.981Z
Learning: The user rbqks529 prefers to extract hardcoded UI strings to StringResource files (strings.xml) rather than keeping them inline in Compose components, which is a good practice for maintainability and localization in Android development.
📚 Learning: 2025-07-01T07:19:10.981Z
Learnt from: rbqks529
PR: THIP-TextHip/THIP-Android#30
File: app/src/main/java/com/texthip/thip/ui/common/cards/CardItemRoomSmall.kt:108-112
Timestamp: 2025-07-01T07:19:10.981Z
Learning: The user rbqks529 prefers to extract hardcoded UI strings to StringResource files (strings.xml) rather than keeping them inline in Compose components, which is a good practice for maintainability and localization in Android development.
Applied to files:
app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.ktapp/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.ktapp/src/main/java/com/texthip/thip/ui/group/room/screen/GroupRoomRecruitScreen.kt
📚 Learning: 2025-07-01T07:19:01.239Z
Learnt from: rbqks529
PR: THIP-TextHip/THIP-Android#30
File: app/src/main/java/com/texthip/thip/ui/myPage/groupPage/GroupPageScreen.kt:34-37
Timestamp: 2025-07-01T07:19:01.239Z
Learning: GroupPageScreen의 MyPageViewModel은 임시 ViewModel로, 현재는 프로토타입 단계이므로 오류 처리 등의 추가 기능은 나중에 구현 예정입니다.
Applied to files:
app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.ktapp/src/main/java/com/texthip/thip/ui/navigator/navigations/GroupNavigation.ktapp/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.ktapp/src/main/java/com/texthip/thip/ui/group/myroom/screen/GroupMyScreen.ktapp/src/main/java/com/texthip/thip/ui/group/screen/GroupScreen.ktapp/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt
🧬 Code Graph Analysis (3)
app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt (4)
app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt (1)
updateState(33-35)app/src/main/java/com/texthip/thip/ui/group/done/viewmodel/GroupDoneViewModel.kt (1)
updateState(27-29)app/src/main/java/com/texthip/thip/ui/group/room/viewmodel/GroupRoomRecruitViewModel.kt (1)
updateState(29-31)app/src/main/java/com/texthip/thip/ui/group/myroom/viewmodel/GroupMyViewModel.kt (1)
updateState(26-28)
app/src/main/java/com/texthip/thip/ui/group/screen/GroupScreen.kt (2)
app/src/main/java/com/texthip/thip/ui/common/buttons/FloatingButton.kt (1)
FloatingButton(25-60)app/src/main/java/com/texthip/thip/ui/common/modal/ToastWithDate.kt (1)
ToastWithDate(24-60)
app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt (1)
app/src/main/java/com/texthip/thip/ui/group/done/viewmodel/GroupDoneViewModel.kt (2)
updateState(27-29)loadUserName(40-47)
🪛 detekt (1.23.8)
app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt
[warning] 82-82: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🔇 Additional comments (11)
app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt (3)
27-27: Hilt ViewModel 주입 전환 LGTM
hiltViewModel()로의 전환과 기본 파라미터 제공은 화면 사용성을 높이고 테스트 시 대체 주입도 용이합니다.Also applies to: 54-55
65-92: UI/로직 분리(GroupMakeRoomContent 도입) 좋습니다상태/이벤트를 명확히 분리하여 재사용성과 테스트 용이성이 향상되었습니다. 콜백 시그니처도 일관적입니다.
247-260: BottomSheet 콜백/상태 연동 정리 LGTM닫기/선택/요청 흐름이 명확해졌고,
isLoadingBooks플래그 반영으로 UX 일관성이 좋아졌습니다.app/src/main/java/com/texthip/thip/ui/navigator/navigations/GroupNavigation.kt (1)
150-160: Back-stack 엔트리 존재 여부 확인 로직 추가 권장
getBackStackEntry(MainTabRoutes.Group)호출은Group탭이 백스택에 없을 때IllegalArgumentException을 던집니다. 예외 발생 시 앱이 크래시될 수 있으므로 존재 여부를 확인하거나runCatching으로 감싸 안전하게 처리해 주세요.app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt (3)
33-39: updateState/DATE_FORMATTER 도입 일관성 좋습니다다른 ViewModel들과 동일한 패턴 유지로 가독성과 유지보수성이 좋아졌습니다.
167-175: 에러 메시지 문자열 리소스 사용 👍하드코딩 제거하고 문자열 리소스와 포맷팅을 사용한 점 좋습니다. 로컬라이제이션 및 유지보수에 유리합니다.
153-166: 중복 검증 제거 제안
GroupMakeRoomUiState.isFormValid에 이미 다음 검증이 포함되어 있습니다:
- 날짜 범위 유효성(
isDurationValid): 시작일이 종료일 이후이거나 두 날짜 간격이 1~90일 범위를 벗어나면false- 비공개 방 비밀번호 유효성(
isPasswordValid):isPrivate == true일 때 비밀번호 길이가 4여야true따라서
createGroup내부에 별도로 시작일-종료일 순서 체크나 비밀번호 유효성 재검사를 추가할 필요가 없습니다.
원하신다면 에러 메시지를 세분화해 다르게 노출하는 식으로만 보완하시면 됩니다.Likely an incorrect or invalid review comment.
app/src/main/java/com/texthip/thip/ui/group/room/screen/GroupRoomRecruitScreen.kt (4)
65-67: roomId 로딩 트리거 적절합니다LaunchedEffect(roomId)로 상세 로딩 트리거한 구성은 명확하고 안전합니다.
104-115: 로딩/데이터 없음 조기 반환 패턴 적절합니다return@Box로 단순/명확하게 분기 처리한 구성 좋습니다.
Also applies to: 117-117
379-386: 하단 버튼 상태/텍스트 파생 로직 분리 양호ButtonType 파생/분기와 문자열 리소스 연결이 명확합니다. 클릭 핸들러를 ViewModel로 위임한 것도 일관성 있습니다.
Also applies to: 388-425
428-436: 토스트 표시/자동 숨김 제어 로직 깔끔합니다네비게이션 진행 중에는 토스트 자동 숨김을 지연시키는 조건도 적절합니다.
Also applies to: 456-463
| suspend fun getRoomRecruiting(roomId: Int): Result<RoomRecruitingResponse> = runCatching { | ||
| groupService.getRoomRecruiting(roomId) | ||
| .handleBaseResponse() | ||
| .getOrThrow()!! | ||
| } |
There was a problem hiding this comment.
널 가능성 방어 코드 필요
getOrThrow()!! 후 강제 언래핑은 서버가 null을 반환할 경우 NPE를 유발합니다. ?: return Result.failure(...) 형태로 안전하게 처리해 주세요.
🤖 Prompt for AI Agents
In app/src/main/java/com/texthip/thip/data/repository/GroupRepository.kt around
lines 65 to 69, the code force unwraps a nullable value with `getOrThrow()!!`,
which can cause a NullPointerException if the server returns null. Modify the
code to safely handle null by using the Elvis operator `?:` to return a
`Result.failure(...)` when the value is null instead of force unwrapping.
| modifier = Modifier.width(18.dp), | ||
| genres = genres, | ||
| genres = uiState.genres.toDisplayStrings(), | ||
| selectedIndex = uiState.selectedGenreIndex, | ||
| onSelect = viewModel::selectGenre | ||
| onSelect = onSelectGenre | ||
| ) |
There was a problem hiding this comment.
GenreChipRow에 18dp 고정 폭 지정으로 레이아웃 클리핑 발생
Modifier.width(18.dp)로 인해 칩들이 보이지 않거나 심하게 잘립니다. 행 전체 폭을 사용하도록 수정하세요.
- GenreChipRow(
- modifier = Modifier.width(18.dp),
+ GenreChipRow(
+ modifier = Modifier.fillMaxWidth(),
genres = uiState.genres.toDisplayStrings(),
selectedIndex = uiState.selectedGenreIndex,
onSelect = onSelectGenre
)📝 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.
| modifier = Modifier.width(18.dp), | |
| genres = genres, | |
| genres = uiState.genres.toDisplayStrings(), | |
| selectedIndex = uiState.selectedGenreIndex, | |
| onSelect = viewModel::selectGenre | |
| onSelect = onSelectGenre | |
| ) | |
| GenreChipRow( | |
| modifier = Modifier.fillMaxWidth(), | |
| genres = uiState.genres.toDisplayStrings(), | |
| selectedIndex = uiState.selectedGenreIndex, | |
| onSelect = onSelectGenre | |
| ) |
🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt
around lines 152 to 156, the use of Modifier.width(18.dp) on GenreChipRow causes
layout clipping and chips are not fully visible. Remove the fixed width modifier
and adjust the layout to use the full available width of the row instead,
ensuring chips are fully displayed without clipping.
| private fun loadGenres() { | ||
| viewModelScope.launch { | ||
| groupRepository.getGenres() | ||
| .onSuccess { genresList -> | ||
| updateState { it.copy(genres = genresList) } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
장르 로딩 실패 처리(onFailure) 및 로깅 보강 필요
현재 onSuccess만 처리하고 onFailure 시 아무 동작이 없어 원인 추적이 어렵습니다. 기본값 세팅과 로깅을 추가하세요.
private fun loadGenres() {
viewModelScope.launch {
groupRepository.getGenres()
.onSuccess { genresList ->
updateState { it.copy(genres = genresList) }
}
+ .onFailure { e ->
+ // TODO: 로깅 프레임워크 적용 시 교체
+ android.util.Log.w("GroupMakeRoomViewModel", "Failed to load genres", e)
+ updateState { it.copy(genres = emptyList()) }
+ }
}
}📝 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.
| private fun loadGenres() { | |
| viewModelScope.launch { | |
| groupRepository.getGenres() | |
| .onSuccess { genresList -> | |
| updateState { it.copy(genres = genresList) } | |
| } | |
| } | |
| } | |
| private fun loadGenres() { | |
| viewModelScope.launch { | |
| groupRepository.getGenres() | |
| .onSuccess { genresList -> | |
| updateState { it.copy(genres = genresList) } | |
| } | |
| .onFailure { e -> | |
| // TODO: 로깅 프레임워크 적용 시 교체 | |
| android.util.Log.w("GroupMakeRoomViewModel", "Failed to load genres", e) | |
| updateState { it.copy(genres = emptyList()) } | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt
around lines 45 to 52, the loadGenres function handles only the success case of
getGenres but lacks failure handling. Add an onFailure block to handle errors by
setting a default value for genres and logging the error to aid in
troubleshooting.
app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt
Show resolved
Hide resolved
| val endDate: Int? = null, // 남은 일 수 | ||
| val imageRes: Int? = R.drawable.bookcover_sample, | ||
| val genreIndex: Int, // 장르 인덱스 | ||
| val imageUrl: String? = null, // API에서 받은 이미지 URL |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
imageUrl 전환 LGTM
리소스 ID에서 URL 기반으로의 전환은 API 연동 방향과 일치합니다. 다만 이 데이터 클래스가 실제 화면/검색에서 사용된다면 mock 패키지에서 도메인/공용 패키지로의 이동을 고려해 주세요.
잔여 imageRes 사용처가 남아있는지 확인하려면 다음 스크립트를 실행해 주세요:
🏁 Script executed:
#!/bin/bash
# imageRes 식별자 잔여 참조 검색
rg -n --hidden --glob '!*build*' 'imageRes'Length of output: 1132
imageResId 사용 잔여 확인 및 통일 필요
아래 파일들에서 여전히 imageResId/imageRes가 사용되고 있습니다. 전체 코드베이스 차원에서 URL 기반 이미지 처리로 일관시켜야 한다면, 해당 부분들도 함께 전환하거나 공용 데이터 클래스로 추출하는 리팩토링을 검토해주세요.
- app/src/main/java/com/texthip/thip/ui/signin/screen/SignupGenreScreen.kt:127
- app/src/main/java/com/texthip/thip/ui/mypage/screen/MypageEditScreen.kt:141
- app/src/main/java/com/texthip/thip/ui/mypage/component/RoleCard.kt:46, 78, 120, 130
- app/src/main/java/com/texthip/thip/ui/mypage/mock/RoleItem.kt:8
- app/src/main/java/com/texthip/thip/ui/common/cards/CardInputBook.kt:37, 51
🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/group/myroom/mock/GroupCardItemRoomData.kt
at line 10, the code uses imageUrl for image handling, but other files still use
imageResId or imageRes. To maintain consistency with URL-based image handling,
locate all instances of imageResId/imageRes in the listed files and refactor
them to use imageUrl instead. Consider extracting a shared data class for image
information to unify image handling across the codebase.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check specific files mentioned in my comment for imageRes/imageResId usage
echo "=== Checking files mentioned in the comment ==="
files=(
"app/src/main/java/com/texthip/thip/ui/signin/screen/SignupGenreScreen.kt"
"app/src/main/java/com/texthip/thip/ui/mypage/screen/MypageEditScreen.kt"
"app/src/main/java/com/texthip/thip/ui/mypage/component/RoleCard.kt"
"app/src/main/java/com/texthip/thip/ui/mypage/mock/RoleItem.kt"
"app/src/main/java/com/texthip/thip/ui/common/cards/CardInputBook.kt"
)
for file in "${files[@]}"; do
if [ -f "$file" ]; then
echo ""
echo "File: $file"
grep -n "imageRes\|imageResId" "$file" || echo " No imageRes/imageResId found"
else
echo ""
echo "File: $file - NOT FOUND"
fi
done
echo ""
echo "=== Checking if these files were modified in this PR ==="
git diff --name-only origin/main...HEAD | grep -E "(SignupGenreScreen|MypageEditScreen|RoleCard|RoleItem|CardInputBook)" || echo "None of these files were modified in this PR"
Length of output: 3397
@rbqks529 네, 맞습니다! 현재 파일(GroupCardItemRoomData.kt)은 이미 올바르게 수정되었습니다. imageRes에서 imageUrl로 잘 전환되었네요.
제가 언급한 다른 파일들(SignupGenreScreen.kt, MypageEditScreen.kt 등)은 이번 PR 범위 밖의 파일들입니다. 이 PR에서는 그룹/모임방 관련 파일들만 수정하셨고, 제가 언급한 파일들은 회원가입이나 마이페이지 관련 파일들이라 별도 작업이 필요한 부분입니다.
현재 변경사항은 문제없이 잘 처리되었습니다! 👍
🧠 Learnings used
Learnt from: rbqks529
PR: THIP-TextHip/THIP-Android#30
File: app/src/main/java/com/texthip/thip/ui/common/cards/CardItemRoomSmall.kt:108-112
Timestamp: 2025-07-01T07:19:10.981Z
Learning: The user rbqks529 prefers to extract hardcoded UI strings to StringResource files (strings.xml) rather than keeping them inline in Compose components, which is a good practice for maintainability and localization in Android development.
| val filtered = roomList.filter { room -> | ||
| (searchText.isBlank() || room.title.contains(searchText, ignoreCase = true)) && | ||
| (selectedGenreIndex == -1 || room.genreIndex == selectedGenreIndex) | ||
| (searchText.isBlank() || room.title.contains(searchText, ignoreCase = true)) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
장르 필터링 책임 위치 재확인 필요(ResultCount 불일치 가능성)
장르 조건이 제거되어 filteredRoomList는 제목 조건만 반영합니다. 만약 장르 필터를 GroupFilteredSearchResult 내부에서 수행한다면, 현재 resultCount = filteredRoomList.size가 실제 화면에 보이는 개수와 불일치할 수 있습니다. 책임 위치를 명확히 해 주세요(내부에서 필터링한다면 count도 내부 결과 기준으로 전달).
장르 필터가 어디서 적용되는지 확인하는 스크립트:
🏁 Script executed:
#!/bin/bash
# GroupFilteredSearchResult가 장르 필터링을 수행하는지 확인
rg -n "Composable\\s+fun\\s+GroupFilteredSearchResult\\(" -A 200
rg -n "selectedGenreIndex|onGenreSelect|genres" -A 20 app/src/main/javaLength of output: 74835
장르 필터링 로직 누락 – 필터 위치 및 카운트 일치화 필요
현재 GroupSearchScreen에서는 filteredRoomList가 텍스트 검색과 정렬만 반영하며, 선택된 장르(selectedGenreIndex)에 대한 필터링이 전혀 적용되지 않아 화면에 보이는 그룹 목록과 resultCount가 일치하지 않습니다.
필터 책임 위치를 명확히 하고, 아래 중 한 가지 방식으로 수정해야 합니다:
-
GroupSearchScreen.kt (약 97–116행)
filteredRoomList생성 시roomList.filter { … }내부에 장르 조건 추가
예).filter { room -> (searchText.isBlank() || room.title.contains(searchText, ignoreCase = true)) && (selectedGenreIndex < 0 || room.genreIndex == selectedGenreIndex) }
-
또는 GroupFilteredSearchResult.kt (컴포저블 내부)
- 전달받은
roomList를selectedGenreIndex기준으로 필터링 후 화면에 표시 resultCount역시 내부 필터링 결과 크기로 재계산
- 전달받은
위 중 선택한 책임 위치에 맞게 로직을 한 곳에 집중시키고, resultCount와 실제 노출되는 아이템 개수가 항상 일치하도록 보장해 주세요.
🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/group/search/screen/GroupSearchScreen.kt
around line 116, the filteredRoomList currently filters only by searchText and
sorting but lacks filtering by selectedGenreIndex, causing a mismatch with
resultCount. To fix this, update the filter logic inside the roomList.filter
block to include a genre condition that checks if selectedGenreIndex is less
than 0 or matches the room's genreIndex, ensuring both the displayed list and
resultCount reflect the selected genre filter consistently.
| viewModelScope.launch { | ||
| repository.getMyRoomGroups() | ||
| .onSuccess { groups -> | ||
| _myRoomGroups.value = groups | ||
| } | ||
| updateState { it.copy(isRefreshing = true) } | ||
| try { | ||
| val jobs = listOf( | ||
| async { loadUserName() }, | ||
| async { | ||
| resetMyGroupsData() | ||
| loadPageBatchSuspend() | ||
| }, | ||
| async { loadRoomSections() }, | ||
| ) | ||
|
|
||
| jobs.awaitAll() | ||
| } finally { | ||
| updateState { it.copy(isRefreshing = false) } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
병렬 로딩 로직이 실제로 대기하지 않음
refreshGroupData에서 async { loadUserName() } 등은 내부에서 또 다른 코루틴을 launch하므로 awaitAll()이 끝나도 작업이 완료되지 않습니다. loadUserName()·loadRoomSections()를 suspend 함수로 변경하거나 repository 호출을 직접 async 블록 안에서 수행하세요.
🤖 Prompt for AI Agents
In app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt
around lines 159 to 174, the async blocks inside refreshGroupData launch
coroutines that internally launch other coroutines, so awaitAll() does not wait
for their completion. To fix this, refactor loadUserName() and
loadRoomSections() to be suspend functions that perform their work directly
without launching new coroutines, or move the repository calls directly inside
the async blocks to ensure awaitAll() properly waits for all tasks to finish.
| val groupViewModel: GroupViewModel = hiltViewModel() | ||
| val uiState by groupViewModel.uiState.collectAsState() | ||
|
|
There was a problem hiding this comment.
사용하지 않는 ViewModel 주입 제거 필요
GroupSearch 컴포저블에서 GroupViewModel을 주입하지만 실제로 UI에 전달하거나 사용하지 않고 있습니다. 불필요한 의존성 주입은 메모리 낭비이므로 제거하거나 활용하도록 수정해 주세요.
🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/navigator/navigations/GroupNavigation.kt
around lines 125 to 127, the GroupViewModel is injected but not used or passed
to the UI, causing unnecessary dependency injection and memory usage. Remove the
injection of GroupViewModel and the related uiState collection if they are not
used in the GroupSearch composable to clean up the code and avoid unused
dependencies.
| fun isRecruitingByType(type: String): Boolean { | ||
| return when (type) { | ||
| "recruiting" -> true | ||
| "playingAndRecruiting" -> false | ||
| "playing" -> false | ||
| "expired" -> false | ||
| else -> false | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
문자열 기반 분기 대신 타입 안전한 모델로 전환 권장 + playingAndRecruiting 의미 재확인 필요
- 하드코딩된 문자열 비교는 오타/계약 변경에 취약합니다. enum/Sealed class 또는 상수로 중앙집중화하세요.
- 또한
"playingAndRecruiting"은 네이밍상 모집 중(true)으로 판단하는 것이 자연스러워 보입니다. 현재 false로 분기되어 있어 비즈니스 규칙을 한 번 더 확인해 주세요.
예시(상수화 + 의미 명확화):
object RoomUtils {
- fun isRecruitingByType(type: String): Boolean {
- return when (type) {
- "recruiting" -> true
- "playingAndRecruiting" -> false
- "playing" -> false
- "expired" -> false
- else -> false
- }
- }
+ object RoomType {
+ const val RECRUITING = "recruiting"
+ const val PLAYING_AND_RECRUITING = "playingAndRecruiting"
+ const val PLAYING = "playing"
+ const val EXPIRED = "expired"
+ }
+
+ fun isRecruitingByType(type: String): Boolean = when (type) {
+ RoomType.RECRUITING -> true
+ RoomType.PLAYING_AND_RECRUITING -> false // ← 비즈니스 규칙에 따라 true가 맞는지 확인 필요
+ RoomType.PLAYING, RoomType.EXPIRED -> false
+ else -> false
+ }하드코딩된 문자열 대신 중앙집중형 타입 모델로 전환 + playingAndRecruiting 분기 로직 재검토 필요
- 대상:
app/src/main/java/com/texthip/thip/utils/rooms/RoomUtils.kt(5–13행) - 현재
isRecruitingByType에서 문자열을 직접 비교하고 있어 오타나 계약 변경에 취약합니다. - 특히
"playingAndRecruiting"은 네이밍상 모집 중인 상태로true가 더 자연스러울 수 있으니, 비즈니스 규칙을 한 번 더 확인해 주세요.
제안 리팩토링 예시:
object RoomUtils {
- fun isRecruitingByType(type: String): Boolean {
- return when (type) {
- "recruiting" -> true
- "playingAndRecruiting" -> false
- "playing" -> false
- "expired" -> false
- else -> false
- }
- }
+ object RoomType {
+ const val RECRUITING = "recruiting"
+ const val PLAYING_AND_RECRUITING = "playingAndRecruiting"
+ const val PLAYING = "playing"
+ const val EXPIRED = "expired"
+ }
+
+ fun isRecruitingByType(type: String): Boolean = when (type) {
+ RoomType.RECRUITING -> true
+ RoomType.PLAYING_AND_RECRUITING -> false // ← 실제 의도대로 true/false 확인 필요
+ RoomType.PLAYING, RoomType.EXPIRED -> false
+ else -> false
+ }
}📝 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.
| fun isRecruitingByType(type: String): Boolean { | |
| return when (type) { | |
| "recruiting" -> true | |
| "playingAndRecruiting" -> false | |
| "playing" -> false | |
| "expired" -> false | |
| else -> false | |
| } | |
| } | |
| object RoomUtils { | |
| object RoomType { | |
| const val RECRUITING = "recruiting" | |
| const val PLAYING_AND_RECRUITING = "playingAndRecruiting" | |
| const val PLAYING = "playing" | |
| const val EXPIRED = "expired" | |
| } | |
| fun isRecruitingByType(type: String): Boolean = when (type) { | |
| RoomType.RECRUITING -> true | |
| RoomType.PLAYING_AND_RECRUITING -> false // ← 실제 의도대로 true/false 확인 필요 | |
| RoomType.PLAYING, RoomType.EXPIRED -> false | |
| else -> false | |
| } | |
| } |
🤖 Prompt for AI Agents
In app/src/main/java/com/texthip/thip/utils/rooms/RoomUtils.kt lines 5 to 13,
replace the hardcoded string comparisons in isRecruitingByType with a
centralized enum or sealed class representing the room types to avoid typos and
ease maintenance. Also, review and correct the logic for the
"playingAndRecruiting" case to ensure it returns true if it represents a
recruiting state according to business rules.
| fun getEndDateInDays(endDate: String): Int { | ||
| return DateUtils.extractDaysFromDeadline(endDate) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
마감일 파싱 실패 시 대비한 안전한 API 제공 제안(Int? or Result 타입)
현재는 파싱 실패 시 예외 전파 가능성이 있습니다. 호출측에서 안전하게 사용할 수 있도록 세이프 오버로드를 추가하는 것을 권장합니다.
object RoomUtils {
fun getEndDateInDays(endDate: String): Int {
return DateUtils.extractDaysFromDeadline(endDate)
}
+
+ // 파싱 실패 시 null 반환하는 세이프 API
+ fun tryGetEndDateInDays(endDate: String): Int? =
+ runCatching { DateUtils.extractDaysFromDeadline(endDate) }.getOrNull()
}🤖 Prompt for AI Agents
In app/src/main/java/com/texthip/thip/utils/rooms/RoomUtils.kt around lines 15
to 17, the getEndDateInDays function currently returns an Int directly, which
may throw exceptions if date parsing fails. To fix this, create a safe overload
or alternative function that returns a nullable Int or a Result type, handling
parsing failures gracefully without throwing exceptions, so callers can safely
handle errors.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/src/main/java/com/texthip/thip/ui/group/room/screen/GroupRoomRecruitScreen.kt (1)
119-126: 배경 이미지를 동적으로 로드하고 접근성을 개선하세요
- 모집 방마다
detail.roomImageUrl이 준비돼 있으므로 고정 drawable 대신 URL 기반AsyncImage를 사용하면 UI 일관성이 높아집니다.- 해당 이미지는 장식용(background) 역할이므로 TalkBack에 불필요한 정보를 주지 않도록
contentDescription = null로 설정하거나stringResource로 분리해 주세요.예시 패치:
-import androidx.compose.foundation.Image +import coil.compose.AsyncImage ... -Image( - painter = painterResource(id = R.drawable.group_room_recruiting), - contentDescription = "배경 이미지", +AsyncImage( + model = detail.roomImageUrl, + contentDescription = null, // decorative
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/src/main/java/com/texthip/thip/data/di/ServiceModule.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/common/cards/CardItemRoom.kt(5 hunks)app/src/main/java/com/texthip/thip/ui/common/cards/CardItemRoomSmall.kt(4 hunks)app/src/main/java/com/texthip/thip/ui/common/cards/CardRoomBook.kt(3 hunks)app/src/main/java/com/texthip/thip/ui/group/room/screen/GroupRoomRecruitScreen.kt(14 hunks)app/src/main/java/com/texthip/thip/ui/mypage/component/SavedFeedCard.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/navigator/navigations/GroupNavigation.kt(6 hunks)app/src/main/res/values/strings.xml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- app/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (6)
- app/src/main/java/com/texthip/thip/ui/mypage/component/SavedFeedCard.kt
- app/src/main/java/com/texthip/thip/data/di/ServiceModule.kt
- app/src/main/java/com/texthip/thip/ui/common/cards/CardItemRoomSmall.kt
- app/src/main/java/com/texthip/thip/ui/common/cards/CardRoomBook.kt
- app/src/main/java/com/texthip/thip/ui/common/cards/CardItemRoom.kt
- app/src/main/java/com/texthip/thip/ui/navigator/navigations/GroupNavigation.kt
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: rbqks529
PR: THIP-TextHip/THIP-Android#30
File: app/src/main/java/com/texthip/thip/ui/common/cards/CardItemRoomSmall.kt:108-112
Timestamp: 2025-07-01T07:19:10.981Z
Learning: The user rbqks529 prefers to extract hardcoded UI strings to StringResource files (strings.xml) rather than keeping them inline in Compose components, which is a good practice for maintainability and localization in Android development.
📚 Learning: 2025-07-01T07:19:10.981Z
Learnt from: rbqks529
PR: THIP-TextHip/THIP-Android#30
File: app/src/main/java/com/texthip/thip/ui/common/cards/CardItemRoomSmall.kt:108-112
Timestamp: 2025-07-01T07:19:10.981Z
Learning: The user rbqks529 prefers to extract hardcoded UI strings to StringResource files (strings.xml) rather than keeping them inline in Compose components, which is a good practice for maintainability and localization in Android development.
Applied to files:
app/src/main/java/com/texthip/thip/ui/group/room/screen/GroupRoomRecruitScreen.kt
➕ 이슈 링크
🔎 작업 내용
코드 변경 사항
API 연결 진행 상황
📸 스크린샷
😢 해결하지 못한 과제
아직 모든 그룹 API 연결을 진행하지는 않았습니다. 또한, 아직 더미데이터가 추가되지 않아 모임방 생성이 가능해지면 나머지도 연결 가능할 것 같습니다.
📢 리뷰어들에게
Summary by CodeRabbit
신규 기능
기능 개선
버그 수정
문서화
리팩터
기타