-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Refactor/#162] ReviewService 부분 Hilt+Flow 리팩토링 #170
Conversation
f7a0970
to
93c5313
Compare
6b689e5
to
eb19b20
Compare
a8caa1c
to
e0b0bdc
Compare
Caution Review failedThe pull request is closed. Walkthrough이 변경 사항은 Android 애플리케이션의 여러 파일에서 발생한 구조적 수정과 기능 변경을 포함합니다. 주요 변경 사항으로는 Changes
Assessment against linked issues
Warning Rate limit exceeded@HI-JIN2 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 39 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 37
🧹 Outside diff range and nitpick comments (51)
app/src/main/java/com/eatssu/android/data/usecase/DeleteReviewUseCase.kt (3)
8-10
: 클래스 선언과 생성자가 잘 구현되었습니다.
DeleteReviewUseCase
클래스의 이름이 그 목적을 명확히 나타내고 있으며, Hilt를 사용한 의존성 주입이 올바르게 구현되었습니다.ReviewRepository
에 대한 의존성도 이 유스케이스에 적합합니다.가독성을 높이기 위해 생성자 매개변수에
private
키워드를 추가하는 것이 좋습니다:-class DeleteReviewUseCase @Inject constructor( - private val reviewRepository: ReviewRepository, +class DeleteReviewUseCase @Inject constructor( + private val reviewRepository: ReviewRepository ) {
11-13
:invoke
함수가 적절히 구현되었습니다.
suspend
키워드와Flow
의 사용이 비동기 작업에 적합하며, 함수 시그니처가 단일 책임 원칙을 잘 따르고 있습니다. Clean Architecture 맥락에서 리포지토리에 작업을 위임하는 것도 올바른 접근입니다.오류 처리를 개선하기 위해
try-catch
블록을 추가하는 것을 고려해보세요:suspend operator fun invoke(reviewId: Long): Flow<BaseResponse<Void>> = try { reviewRepository.deleteReview(reviewId) } catch (e: Exception) { flowOf(BaseResponse.Error(e.message ?: "알 수 없는 오류가 발생했습니다.")) }이렇게 하면 예기치 않은 예외를 잡아 클라이언트에게 더 명확한 오류 메시지를 제공할 수 있습니다.
1-13
: 전반적으로 우수한 구현입니다.이
DeleteReviewUseCase
클래스는 Clean Architecture 원칙을 잘 따르고 있으며, 단일 책임에 집중된 간결한 구현을 보여줍니다. 최신 Kotlin 기능과 코루틴을 효과적으로 활용하고 있어 좋습니다.클래스와 함수에 KDoc 주석을 추가하여 문서화를 개선하는 것을 추천합니다. 예를 들어:
/** * 리뷰 삭제를 위한 유스케이스. * * @property reviewRepository 리뷰 관련 작업을 수행하는 리포지토리 */ class DeleteReviewUseCase @Inject constructor( private val reviewRepository: ReviewRepository ) { /** * 주어진 ID의 리뷰를 삭제합니다. * * @param reviewId 삭제할 리뷰의 ID * @return 삭제 작업의 결과를 나타내는 Flow */ suspend operator fun invoke(reviewId: Long): Flow<BaseResponse<Void>> = reviewRepository.deleteReview(reviewId) }이렇게 문서화를 추가하면 코드의 가독성과 유지보수성이 향상됩니다.
app/src/main/java/com/eatssu/android/data/usecase/WriteReviewUseCase.kt (2)
9-11
: LGTM: 클래스 선언과 생성자가 잘 구현되었습니다.
WriteReviewUseCase
클래스의 구조가 Clean Architecture 원칙을 잘 따르고 있습니다. Hilt를 사용한 의존성 주입이 올바르게 구현되었고, 생성자 매개변수가 private으로 선언되어 캡슐화가 잘 되어 있습니다.클래스에 간단한 KDoc 주석을 추가하여 이 use case의 목적을 명확히 설명하는 것이 좋을 것 같습니다. 예를 들어:
/** * UseCase for writing a review for a specific menu item. */ class WriteReviewUseCase @Inject constructor( private val reviewRepository: ReviewRepository, ) { // ... }
12-14
: LGTM: invoke 메서드가 잘 구현되었습니다.
invoke
메서드의 구현이 Clean Architecture 원칙을 잘 따르고 있습니다. suspend 함수와 Flow를 사용하여 비동기 작업을 적절히 처리하고 있으며, operator 함수 문법을 사용하여 호출 부분의 가독성을 높였습니다.다음과 같은 개선 사항을 고려해 보시기 바랍니다:
- 메서드에 KDoc 주석을 추가하여 파라미터와 반환값에 대한 설명을 제공하는 것이 좋습니다.
- 에러 처리 로직을 추가하는 것을 고려해 보세요. 예를 들어, 리포지토리에서 발생할 수 있는 예외를 캐치하고 적절한 에러 응답으로 변환할 수 있습니다.
예시:
/** * Writes a review for a specific menu item. * * @param menuId The ID of the menu item being reviewed. * @param body The content of the review. * @return A Flow emitting the result of the review submission. */ suspend operator fun invoke(menuId: Long, body: WriteReviewRequest): Flow<BaseResponse<Void>> = try { reviewRepository.writeReview(menuId, body) } catch (e: Exception) { flow { emit(BaseResponse.Error(e.message ?: "Unknown error occurred")) } }이러한 변경은 메서드의 동작을 더 명확히 하고, 예외 상황을 더 잘 처리할 수 있게 해줍니다.
app/src/main/java/com/eatssu/android/data/usecase/GetMealReviewInfoUseCase.kt (2)
9-11
: 클래스 선언과 생성자가 잘 구현되었습니다.클래스 이름이 용도에 맞게 잘 지어졌고, Hilt를 사용한 의존성 주입이 올바르게 구현되었습니다.
reviewRepository
를 private으로 선언한 것도 좋습니다.개선 제안: 클래스에 간단한 KDoc 주석을 추가하여 이 UseCase의 목적을 명확히 설명하는 것이 좋겠습니다. 예를 들면:
/** * 식사 리뷰 정보를 가져오는 UseCase. */ class GetMealReviewInfoUseCase @Inject constructor( private val reviewRepository: ReviewRepository, ) { // ... }
12-14
: invoke 메서드가 잘 구현되었습니다.
operator fun invoke
를 사용하여 클래스를 함수처럼 호출할 수 있게 한 것이 좋습니다. 메서드 시그니처가 클린 아키텍처 원칙에 잘 부합하며, 리포지토리 메서드로의 직접적인 위임도 이 간단한 유스케이스에 적절합니다.개선 제안: 에러 처리를 위해 try-catch 블록을 추가하는 것이 좋겠습니다. 예를 들면:
suspend operator fun invoke(mealId: Long): Flow<BaseResponse<GetMealReviewInfoResponse>> = try { reviewRepository.getMealReviewInfo(mealId) } catch (e: Exception) { flow { emit(BaseResponse.Error(e.message ?: "알 수 없는 오류가 발생했습니다.")) } }이렇게 하면 리포지토리에서 발생할 수 있는 예외를 캐치하여 적절한 에러 응답을 생성할 수 있습니다.
app/src/main/java/com/eatssu/android/data/usecase/GetMenuReviewInfoUseCase.kt (3)
9-11
: 클래스 정의와 생성자가 잘 구현되었습니다.
GetMenuReviewInfoUseCase
클래스의 정의와 생성자 주입이 Clean Architecture 원칙에 따라 잘 구현되었습니다. 단일 책임 원칙을 준수하고 있으며, Hilt를 통한 의존성 주입이 올바르게 사용되었습니다.가독성을 높이기 위해
reviewRepository
매개변수에private
키워드를 추가하는 것이 좋습니다:class GetMenuReviewInfoUseCase @Inject constructor( private val reviewRepository: ReviewRepository )
12-14
:invoke
메서드가 적절히 구현되었습니다.
invoke
메서드의 구현이 use case 패턴을 잘 따르고 있으며, Kotlin의 연산자 오버로딩 기능을 적절히 활용하고 있습니다. Flow를 사용한 비동기 작업 처리도 적절합니다.오류 처리를 개선하기 위해 다음과 같이 try-catch 블록을 추가하는 것을 고려해보세요:
suspend operator fun invoke(menuId: Long): Flow<BaseResponse<GetMenuReviewInfoResponse>> = try { reviewRepository.getMenuReviewInfo(menuId) } catch (e: Exception) { flow { emit(BaseResponse.Error(e.message ?: "알 수 없는 오류가 발생했습니다.")) } }이렇게 하면 리포지토리에서 발생할 수 있는 예외를 캐치하여 더 견고한 오류 처리가 가능해집니다.
1-14
: 전반적으로 잘 구현된 Use Case입니다.이
GetMenuReviewInfoUseCase
는 Clean Architecture 원칙을 잘 따르고 있으며, Hilt를 통한 의존성 주입과 Kotlin 코루틴을 활용한 비동기 작업 처리가 적절히 이루어져 있습니다. 코드가 간결하고 단일 책임에 집중되어 있어 좋습니다.코드의 유지보수성을 더욱 향상시키기 위해, 클래스와 메서드에 KDoc 주석을 추가하는 것을 고려해보세요. 예를 들어:
/** * 메뉴 리뷰 정보를 가져오는 Use Case * * @property reviewRepository 리뷰 정보를 가져오는 데 사용되는 리포지토리 */ class GetMenuReviewInfoUseCase @Inject constructor( private val reviewRepository: ReviewRepository ) { /** * 지정된 메뉴 ID에 대한 리뷰 정보를 가져옵니다. * * @param menuId 리뷰 정보를 가져올 메뉴의 ID * @return 메뉴 리뷰 정보를 포함한 [BaseResponse]의 [Flow] */ suspend operator fun invoke(menuId: Long): Flow<BaseResponse<GetMenuReviewInfoResponse>> = reviewRepository.getMenuReviewInfo(menuId) }이러한 문서화는 코드의 목적과 사용 방법을 명확히 하는 데 도움이 됩니다.
app/src/main/java/com/eatssu/android/data/usecase/ModifyReviewUseCase.kt (1)
1-17
: 전반적으로 잘 구현된 UseCase, 문서화 제안이 UseCase는 단일 책임 원칙을 잘 따르고 있으며, Clean Architecture의 원칙에 맞게 구현되었습니다. Hilt와 Flow를 사용한 리팩토링 목표에 잘 부합합니다.
개선 제안:
클래스와 invoke 함수에 KDoc 주석을 추가하여 사용 방법과 목적을 명확히 문서화하는 것이 좋겠습니다. 예를 들어:
/** * UseCase for modifying an existing review. */ class ModifyReviewUseCase @Inject constructor( private val reviewRepository: ReviewRepository, ) { /** * Modifies a review with the given ID using the provided request body. * * @param reviewId The ID of the review to modify. * @param body The request body containing the modified review data. * @return A Flow emitting the result of the modification operation. */ suspend operator fun invoke( reviewId: Long, body: ModifyReviewRequest, ): Flow<BaseResponse<Void>> = reviewRepository.modifyReview(reviewId, body) }app/src/main/java/com/eatssu/android/data/usecase/GetImageUrlUseCase.kt (2)
10-12
: 클래스 선언과 생성자가 잘 구현되었습니다.
GetImageUrlUseCase
클래스가 단일 책임 원칙을 잘 따르고 있으며, 의존성 주입을 사용하여 유지보수성과 테스트 용이성을 높였습니다.클래스에 간단한 KDoc 주석을 추가하여 클래스의 목적을 명확히 설명하는 것이 좋겠습니다. 예를 들어:
/** * 이미지 URL을 가져오는 유스케이스 */ class GetImageUrlUseCase @Inject constructor( private val reviewRepository: ReviewRepository, ) { // ... }
13-17
: 연산자 함수가 잘 구현되었습니다.suspend 함수와 Flow의 사용이 비동기 작업에 적합하며, 연산자 함수를 사용하여 클래스를 함수처럼 호출할 수 있게 한 것은 깔끔한 설계입니다.
함수에 KDoc 주석을 추가하여 파라미터와 반환값에 대한 설명을 제공하는 것이 좋겠습니다. 예를 들어:
/** * 이미지 URL을 가져오는 작업을 수행합니다. * * @param image 업로드할 이미지 파일 * @return 이미지 URL을 포함한 응답의 Flow */ suspend operator fun invoke( image: MultipartBody.Part, ): Flow<BaseResponse<ImageResponse>> = reviewRepository.getImageString(image)app/src/main/java/com/eatssu/android/App.kt (1)
Line range hint
13-13
: TODO 주석에 대한 후속 조치가 필요합니다.
appContext
변수를 제거하라는 TODO 주석이 있습니다. 이 변수의 사용 여부를 확인하고, 필요하지 않다면 제거하는 것이 좋겠습니다. 만약 아직 사용 중이라면, 더 나은 대안을 찾아보는 것을 고려해 보세요.app/src/main/java/com/eatssu/android/data/usecase/GetMenuReviewListUseCase.kt (1)
10-12
: 클래스 선언과 생성자가 잘 구현되었습니다.클래스 이름과 구조가 유스케이스 패턴을 잘 따르고 있으며, 의존성 주입이 올바르게 구현되었습니다. 클린 아키텍처 원칙에 따라 리포지토리를 사용하고 있습니다.
가독성을 높이기 위해 생성자 매개변수에
private
키워드를 추가하는 것이 좋습니다:-class GetMenuReviewListUseCase @Inject constructor( - private val reviewRepository: ReviewRepository, +class GetMenuReviewListUseCase @Inject constructor( + private val reviewRepository: ReviewRepository, ) {app/src/main/java/com/eatssu/android/ui/review/report/OthersReviewDialogActivity.kt (1)
32-32
: 로깅 메커니즘이 적절히 업데이트되었습니다.Log.d에서 Timber.d로의 전환이 잘 이루어졌습니다. 로그 메시지의 형식도 개선되어 디버깅이 더 용이해졌습니다.
로그 메시지를 더욱 명확하게 만들기 위해 다음과 같이 수정하는 것은 어떨까요?
Timber.d("Starting ReportActivity with reviewId: $reviewId")이렇게 하면 로그의 컨텍스트가 더 명확해집니다.
app/src/main/java/com/eatssu/android/di/DataModule.kt (2)
15-15
: 별칭 사용 이유 명확화 필요
ReviewRepositoryImpl
에 대한 별칭을 사용하고 있습니다. 이는 잠재적으로 혼란을 야기할 수 있습니다.별칭 사용의 이유를 주석으로 설명하거나, 가능하다면 원래의 이름을 직접 사용하는 것이 좋습니다. 예를 들어:
// 다른 모듈의 ReviewRepositoryImpl과 구분하기 위해 별칭 사용 import com.eatssu.android.data.repository.ReviewRepositoryImpl as ReviewRepositoryImpl1
36-39
: 바인딩 메서드 추가 승인 및 포맷팅 제안
ReviewRepository
에 대한 새로운 바인딩 메서드가 올바르게 추가되었습니다. 구조와 어노테이션 사용이 적절합니다.코드의 일관성을 위해 다른 바인딩 메서드와 같은 형식으로 작성하는 것이 좋습니다. 예를 들어:
@Binds internal abstract fun bindsReviewRepository( reviewRepositoryImpl: ReviewRepositoryImpl1 ): ReviewRepositoryapp/src/main/java/com/eatssu/android/ui/mypage/terms/WebViewActivity.kt (2)
32-32
: Timber를 사용한 로깅으로의 전환을 승인합니다.Log.d에서 Timber.d로의 전환은 적절합니다. 이는 새로운 로깅 프레임워크와 일치하며, 코드의 일관성을 유지합니다.
로그 메시지의 가독성을 높이기 위해 다음과 같이 포맷을 개선하는 것을 제안합니다:
Timber.d("URL: $URL, TITLE: $TITLE")이렇게 하면 로그 출력에서 URL과 TITLE을 더 쉽게 구분할 수 있습니다.
Line range hint
1-48
: 전반적인 변경사항에 대한 승인 및 향후 개선 제안이 파일의 변경사항은 주로 로깅 방식의 개선에 초점을 맞추고 있으며, WebViewActivity의 핵심 기능은 그대로 유지되었습니다. 이는 PR의 리팩토링 목표와 일치하며 긍정적인 변화입니다.
향후 개선을 위한 제안:
TODO 주석에서 언급된 대로 ViewModel을 사용하여 데이터 처리 로직을 분리하는 것을 고려해보세요. 이는 MVVM 아키텍처 원칙에 더 부합할 것입니다.
웹뷰 로딩 실패나 네트워크 오류 등에 대한 추가적인 에러 처리를 구현하는 것이 좋습니다.
URL과 TITLE을 intent로 전달하는 대신, companion object에 정의된 상수를 사용하여 액티비티를 시작하는 팩토리 메서드를 만드는 것을 고려해보세요. 이는 타입 안정성을 높이고 사용의 일관성을 보장할 수 있습니다.
이러한 개선사항들은 코드의 견고성과 유지보수성을 더욱 향상시킬 것입니다.
app/src/main/java/com/eatssu/android/ui/mypage/myreview/MyReviewViewModel.kt (1)
43-43
: Timber를 사용한 디버그 로깅 승인 및 개선 제안Timber.d를 사용한 디버그 로깅은 적절합니다. 그러나 더 명확한 로그 메시지를 위해 다음과 같이 개선할 수 있습니다:
다음과 같이 로그 메시지를 더 구체적으로 만들어 보는 것은 어떨까요?
Timber.d("GetMyReviews result: %s", result.toString())이렇게 하면 로그의 출처와 내용을 더 쉽게 식별할 수 있습니다.
app/src/main/java/com/eatssu/android/ui/review/write/menu/VariableMenuViewModel.kt (2)
41-41
: 로깅 구현 개선 승인 및 제안Android Log에서 Timber로의 전환이 올바르게 이루어졌습니다. 이는 애플리케이션 전반의 로깅 표준화 노력과 일치합니다.
개선 제안:
- 로그 메시지에 더 많은 컨텍스트 정보를 추가하면 디버깅에 도움이 될 수 있습니다. 예를 들어:
Timber.d("findMenuItemByMealId 성공: mealId = $mealId, response = ${response.body()}") Timber.e("findMenuItemByMealId 실패: mealId = $mealId, errorBody = ${response.errorBody()?.string()}") Timber.e("findMenuItemByMealId 네트워크 오류: mealId = $mealId", t)이렇게 하면 문제 발생 시 더 쉽게 추적할 수 있습니다.
Also applies to: 50-50, 58-58
Line range hint
1-71
: 전체 리뷰 요약 및 제안이 파일의 변경사항은 주로 로깅 개선에 초점을 맞추고 있으며, 이는 잘 구현되었습니다. 그러나 에러 처리와 관련하여 개선의 여지가 있습니다:
- 오류 상태 처리: 응답이 실패하거나 네트워크 오류가 발생할 때
_uiState
를 업데이트하여 오류 상태를 반영하는 것이 좋습니다. 예를 들어:_uiState.update { it.copy( loading = false, error = true, menuOfMeal = null ) }
- 에러 메시지: 사용자에게 표시할 수 있는 오류 메시지를
MenuState
에 추가하는 것을 고려해 보세요.이러한 변경사항들은 애플리케이션의 견고성과 사용자 경험을 향상시킬 수 있습니다.
app/src/main/java/com/eatssu/android/di/NetworkModule.kt (1)
85-91
: LGTM! 일관성을 위한 작은 제안이 있습니다.
provideReviewService
메서드의 추가는 모듈의 기존 패턴과 일치하며, PR의 목표인 ReviewService 컴포넌트 리팩토링과 Hilt를 통한 의존성 주입 구현에 부합합니다.일관성을 위해 다음과 같이 메서드 순서를 조정하는 것이 어떨까요?
@Provides @Singleton fun provideReportService(retrofit: Retrofit): ReportService { return retrofit.create(ReportService::class.java) } +@Provides +@Singleton +fun provideReviewService(retrofit: Retrofit): ReviewService { + return retrofit.create(ReviewService::class.java) +} + @Provides @Singleton -fun provideReviewService(retrofit: Retrofit): ReviewService { - return retrofit.create(ReviewService::class.java) +fun provideUserService(retrofit: Retrofit): UserService { + return retrofit.create(UserService::class.java) } - -@Provides -@Singleton -fun provideUserService(retrofit: Retrofit): UserService { - return retrofit.create(UserService::class.java) -}이렇게 하면
ReviewService
가ReportService
바로 다음에 위치하게 되어, 비슷한 기능을 가진 서비스들이 함께 그룹화됩니다.app/src/main/java/com/eatssu/android/ui/review/write/menu/ReviewWriteMenuActivity.kt (3)
53-53
: 로그 메시지 개선을 위한 제안Timber로의 전환은 잘 이루어졌습니다. 로그 메시지를 더욱 명확하게 만들기 위해 다음과 같은 형식을 제안합니다:
Timber.d("받은 메뉴 데이터: %s", it.menuOfMeal.toString())이렇게 하면 로그 메시지의 의도가 더 명확해지고, Timber의 문자열 포매팅 기능을 활용할 수 있습니다.
74-74
: 로그 메시지 내용 개선 제안Timber로의 전환은 잘 이루어졌습니다. 하지만 로그 메시지의 내용을 더 유익하게 만들 수 있습니다. 다음과 같은 형식을 고려해보세요:
Timber.d("sendNextItem: 총 %d개 중 %d번째 아이템 처리 중. 현재 아이템: %s", items.size, i + 1, currentItem)이렇게 하면 총 아이템 수, 현재 처리 중인 아이템의 인덱스, 그리고 현재 아이템의 세부 정보를 한 번에 확인할 수 있어 디버깅에 더 유용할 것입니다.
Line range hint
1-92
: 전체 리뷰 요약 및 향후 개선 제안이 파일의 변경사항은 주로 로깅 방식의 개선에 초점을 맞추고 있으며, 이는 전체적인 리팩토링 노력과 일치합니다. 핵심 기능은 그대로 유지되어 있어 좋습니다.
향후 개선을 위한 제안:
- Hilt를 사용한 의존성 주입 도입: ViewModel 생성 로직을 Hilt를 사용하여 간소화할 수 있습니다.
- Coroutines와 Flow 활용:
lifecycleScope.launch
와collectLatest
를 사용하고 있지만, 이를 더 확장하여 다른 비동기 작업에도 적용할 수 있습니다.- 에러 처리: 현재 에러 상태를 체크하고 있지만, 사용자에게 에러를 표시하는 로직이 없습니다. 이를 개선하여 사용자 경험을 향상시킬 수 있습니다.
전반적으로 잘 진행된 리팩토링입니다. 앞으로의 개선 작업도 기대하겠습니다.
app/src/main/java/com/eatssu/android/ui/mypage/myreview/MyReviewAdapter.kt (2)
37-37
: 로그 메시지 개선 제안현재 로그 메시지는 "사진 있다"로 되어 있습니다. 이 메시지를 더 명확하고 정보를 제공하는 형태로 개선하는 것이 좋겠습니다.
다음과 같이 변경하는 것을 고려해보세요:
Timber.d("리뷰 이미지 URL 존재: ${dataList[position].imgUrl}")이렇게 하면 디버깅 시 어떤 이미지 URL이 사용되는지 더 쉽게 파악할 수 있습니다.
Line range hint
18-82
: 전반적인 개선 사항 제안어댑터의 기능성과 코드 품질을 높이기 위해 다음 개선 사항들을 고려해보세요:
데이터 업데이트 메커니즘:
현재List<Review>
를 사용하고 있어 데이터 업데이트가 어렵습니다.class MyReviewAdapter(private var dataList: MutableList<Review>) : ... fun updateData(newData: List<Review>) { dataList.clear() dataList.addAll(newData) notifyDataSetChanged() }이미지 로딩 로직 간소화:
private fun loadImage(imageUrl: String?) { if (!imageUrl.isNullOrEmpty()) { Timber.d("리뷰 이미지 로딩: $imageUrl") Glide.with(itemView) .load(imageUrl) .into(binding.ivReviewPhoto) binding.ivReviewPhoto.visibility = View.VISIBLE } else { binding.ivReviewPhoto.visibility = View.GONE } }그리고
bind
메소드에서:loadImage(dataList[position].imgUrl?.firstOrNull())이러한 변경으로 코드의 가독성과 유지보수성이 향상될 것입니다.
app/src/main/java/com/eatssu/android/ui/mypage/usernamechange/UserNameChangeViewModel.kt (2)
Line range hint
69-69
:changeNickname
함수에서 Timber 사용 권장
checkNickname
함수에서는 Timber를 사용하도록 변경되었지만,changeNickname
함수에서는 여전히Log.e
를 사용하고 있습니다. 일관성을 위해 이 부분도 Timber를 사용하도록 변경하는 것이 좋습니다.다음과 같이 수정해 주세요:
Timber.e(e.toString())
이렇게 하면 로깅 방식이 일관되어 로그 관리가 더 쉬워질 것입니다.
Line range hint
1-94
: 종합 리뷰 및 다음 단계 제안이 변경사항은 Timber 로깅 라이브러리의 도입과 오류 처리 로직의 개선을 중심으로 이루어졌습니다. 이는 전반적인 로깅 관행 개선 목표에 부합합니다. 그러나 몇 가지 개선이 필요한 부분이 있습니다:
onCompletion
블록에서의 오류 상태 설정 로직을 수정하여 실제 오류 발생 시에만error
상태를true
로 설정하도록 합니다.changeNickname
함수에서도 Timber를 사용하도록 변경하여 로깅 방식의 일관성을 유지합니다.이러한 변경사항을 적용하면 코드의 일관성이 향상되고 오류 처리가 더 정확해질 것입니다. 또한, 전체적인 코드 품질과 유지보수성이 개선될 것입니다.
다음 단계로, 이러한 변경사항을 적용한 후 단위 테스트를 업데이트하고 실행하여 변경된 로직이 예상대로 작동하는지 확인하는 것이 좋습니다. 또한, 이러한 변경사항이 다른 부분의 코드에 미치는 영향을 검토하고, 필요한 경우 관련 문서를 업데이트하는 것도 고려해 보세요.
app/src/main/res/values/strings.xml (2)
75-78
: 리뷰 수정 관련 문자열 리소스 추가 승인리뷰 수정 과정에 대한 새로운 문자열 리소스들이 잘 추가되었습니다. 사용자에게 명확한 안내를 제공합니다.
제안: "modify_not" 문자열을 "리뷰 수정에 실패했습니다."로 변경하는 것이 어떨까요? 이렇게 하면 다른 메시지들과 더 일관된 어조를 유지할 수 있습니다.
80-84
: 리뷰 삭제 관련 문자열 리소스 추가 승인리뷰 삭제 과정에 대한 새로운 문자열 리소스들이 잘 추가되었습니다. 사용자에게 명확한 안내를 제공합니다.
제안:
- "delete" 문자열(80줄)을 "리뷰 삭제"로 변경하여 다른 문자열들과 일관성을 유지하는 것이 어떨까요?
- "delete_not" 문자열(83줄)을 "리뷰 삭제에 실패했습니다."로 변경하여 다른 메시지들과 더 일관된 어조를 유지하는 것이 어떨까요?
- "delete_done" 문자열(82줄)에서 띄어쓰기를 수정하여 "리뷰가 삭제되었습니다."로 변경하는 것이 어떨까요?
app/src/main/java/com/eatssu/android/data/service/ReviewService.kt (1)
40-40
: **사용하지 않는 코드 제거 권장 **주석 처리된
@Query("lastReviewId") lastReviewId: Long?,
라인이 있습니다. 불필요한 코드라면 삭제를 고려해주세요.app/src/main/java/com/eatssu/android/ui/review/delete/DeleteViewModel.kt (2)
43-43
: 오류 로그에 예외 객체 자체를 전달 추천
Timber.e(e.toString())
대신Timber.e(e)
를 사용하면 전체 스택 트레이스를 포함한 상세한 오류 정보를 로그로 남길 수 있어 디버깅에 도움이 됩니다.
59-59
: DeleteState의 'loading' 초기값을 'false'로 설정 제안현재
loading
의 기본값이true
로 설정되어 있어 ViewModel 생성 시점부터 로딩 상태로 표시될 수 있습니다. 일반적으로 로딩은 특정 동작 시작 시에만 표시되므로, 초기값을false
로 설정하고 로딩이 필요한 시점에true
로 변경하는 것이 좋습니다.app/src/main/java/com/eatssu/android/ui/review/modify/ModifyViewModel.kt (1)
19-19
: Timber 로그 사용 검토
import timber.log.Timber
임포트가 추가되었고, 코드에서Timber.e
와Timber.d
를 사용하고 있습니다. 로그를 활용하여 디버깅을 용이하게 하는 것은 좋습니다. 하지만 배포 시 불필요한 로그가 남지 않도록 로그 레벨이나 사용 여부를 검토해주세요.app/src/main/java/com/eatssu/android/ui/review/modify/ModifyReviewActivity.kt (1)
96-96
: TODO 항목을 해결하여 사용자 경험을 개선하세요."쓰다 뒤로 갔을 때 undo" 기능을 구현하면 사용자가 입력한 내용을 실수로 잃어버리는 것을 방지할 수 있습니다.
이 기능 구현을 도와드릴까요? 제가 해당 기능에 대한 구현 방안을 제안하거나 새로운 GitHub 이슈를 생성해 드릴 수 있습니다.
app/src/main/java/com/eatssu/android/ui/review/write/ReviewWriteViewModel.kt (1)
Line range hint
86-90
:UploadReviewState
의 초기loading
값을false
로 설정하는 것을 권장합니다현재
UploadReviewState
의loading
값이 기본적으로true
로 설정되어 있어, ViewModel 생성 시점부터 로딩 상태로 표시될 수 있습니다. 일반적으로 로딩 상태는 특정 작업이 시작될 때true
로 설정하므로, 초기값을false
로 두는 것이 바람직합니다.data class UploadReviewState( var toastMessage: String = "", - var loading: Boolean = true, + var loading: Boolean = false, var error: Boolean = false, var isUpload: Boolean = false, )app/src/main/java/com/eatssu/android/ui/review/list/ReviewActivity.kt (2)
48-48
: TODO 주석 처리 필요'//todo 이거 안하면 바로바로 갱신이 안되는디'라는 주석이 남아 있습니다. 해당 사항이 완료되었다면 주석을 제거해 주세요. 아직 작업 중이라면 구체적인 계획을 세우는 것이 좋습니다.
필요하시면 작업 계획 수립이나 이슈 생성에 도움을 드릴 수 있습니다.
64-64
: TODO 주석 처리 필요'//Todo 여기서는 메뉴 타입이 뭔지 몰라도 됨. 추상화 해도 됨'이라는 주석이 남아 있습니다. 해당 부분을 추상화하려는 계획이라면 구현을 진행하거나 주석을 정리해 주세요.
이 부분에 대해 도움이 필요하시면 말씀해 주세요.
app/src/main/java/com/eatssu/android/ui/review/write/ImageViewModel.kt (2)
124-124
: 예외 로깅 시 불필요한toString()
호출 제거
Timber.e(e.toString())
대신Timber.e(e)
를 사용하면 예외의 스택 트레이스를 포함한 상세 정보를 로그로 남길 수 있습니다.수정된 코드:
.catch { e -> _uiState.update { it.copy(error = true, toastMessage = "이미지 변환에 실패하였습니다.") } - Timber.e(e.toString()) + Timber.e(e) }
133-133
: 이미 문자열인 변수에 대한toString()
호출 제거
url
변수가 이미 문자열(String) 타입이라면url.toString()
은 불필요한 호출입니다. 직접 할당하여 코드를 간결하게 만들 수 있습니다.수정된 코드:
imgUrl = url.toString()를
imgUrl = url로 변경
app/src/main/java/com/eatssu/android/ui/review/list/ReviewViewModel.kt (4)
74-75
: 예외 로그 레벨 수정 권장
catch
블록에서Timber.d(e.toString())
를 사용하여 디버그 수준으로 예외를 로그하고 있습니다. 예외 발생 시에는Timber.e(e)
를 사용하여 에러 수준으로 로그를 남기는 것이 적절합니다. 이렇게 하면 예외 스택 트레이스도 함께 출력되어 디버깅에 도움이 됩니다.수정 예시:
- Timber.d(e.toString()) + Timber.e(e)
117-118
: 예외 로그 레벨 수정 권장
catch
블록에서Timber.e(e.toString())
를 사용하고 있습니다.Timber.e(e)
를 사용하면 예외 스택 트레이스가 포함되어 더 자세한 디버깅 정보를 얻을 수 있습니다.수정 예시:
- Timber.e(e.toString()) + Timber.e(e)
160-161
: 예외 로그 레벨 수정 권장
catch
블록에서Timber.e(e.toString())
를 사용하고 있습니다. 예외를 에러 수준으로 로그하고 스택 트레이스를 포함하기 위해Timber.e(e)
를 사용하는 것이 좋습니다.수정 예시:
- Timber.e(e.toString()) + Timber.e(e)
201-202
: 예외 로그 레벨 수정 권장
catch
블록에서Timber.e(e.toString())
를 사용하고 있습니다.Timber.e(e)
로 변경하여 예외 정보를 더욱 상세하게 로그로 남길 수 있습니다.수정 예시:
- Timber.e(e.toString()) + Timber.e(e)app/src/main/java/com/eatssu/android/ui/review/write/ReviewWriteRateActivity.kt (4)
81-82
: 불필요한 주석 코드 정리 필요사용하지 않는 주석 처리된 코드가 남아 있습니다. 코드의 가독성을 위해 제거하는 것이 좋습니다.
주석 처리된 코드를 삭제하여 정리해주세요.
129-129
: 문자열 연결 방식 개선
Timber.d
로그 출력 시 문자열 연결에+
연산자 대신 문자열 템플릿을 사용하는 것이 가독성에 좋습니다.다음과 같이 수정해주세요:
- Timber.d("압축 된 사이즈+" + (compressedImage?.length()?.div(1024)).toString()) + Timber.d("압축 된 사이즈: ${compressedImage?.length()?.div(1024)}") - Timber.d("Compressed image save in " + getReadableFileSize(it.length())) + Timber.d("Compressed image save in ${getReadableFileSize(it.length())}")Also applies to: 150-150
77-77
: 사용되지 않는 변수 제거 검토
imageService
변수가 선언되었지만 코드 내에서 사용되지 않는 것 같습니다. 불필요한 변수라면 제거하는 것이 좋습니다.사용하지 않는
imageService
변수를 삭제해주세요.
362-363
: TAG 상수 중복 선언 확인
const val TAG = "ReviewWriteRateActivity"
를 선언하였지만,Timber
를 사용할 때 태그를 명시적으로 지정하지 않아도 됩니다. 필요하지 않다면 제거해주세요.
TAG
상수를 삭제하여 코드를 정리해주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (41)
- app/src/main/java/com/eatssu/android/App.kt (1 hunks)
- app/src/main/java/com/eatssu/android/data/repository/ImageRepository.kt (0 hunks)
- app/src/main/java/com/eatssu/android/data/repository/ImageRepositoryImpl.kt (0 hunks)
- app/src/main/java/com/eatssu/android/data/repository/ReviewRepository.kt (1 hunks)
- app/src/main/java/com/eatssu/android/data/repository/ReviewRepositoryImpl.kt (1 hunks)
- app/src/main/java/com/eatssu/android/data/service/ImageService.kt (1 hunks)
- app/src/main/java/com/eatssu/android/data/service/ReviewService.kt (1 hunks)
- app/src/main/java/com/eatssu/android/data/usecase/DeleteReviewUseCase.kt (1 hunks)
- app/src/main/java/com/eatssu/android/data/usecase/GetImageUrlUseCase.kt (1 hunks)
- app/src/main/java/com/eatssu/android/data/usecase/GetMealReviewInfoUseCase.kt (1 hunks)
- app/src/main/java/com/eatssu/android/data/usecase/GetMealReviewListUseCase.kt (1 hunks)
- app/src/main/java/com/eatssu/android/data/usecase/GetMenuReviewInfoUseCase.kt (1 hunks)
- app/src/main/java/com/eatssu/android/data/usecase/GetMenuReviewListUseCase.kt (1 hunks)
- app/src/main/java/com/eatssu/android/data/usecase/ModifyReviewUseCase.kt (1 hunks)
- app/src/main/java/com/eatssu/android/data/usecase/WriteReviewUseCase.kt (1 hunks)
- app/src/main/java/com/eatssu/android/di/DataModule.kt (2 hunks)
- app/src/main/java/com/eatssu/android/di/NetworkModule.kt (2 hunks)
- app/src/main/java/com/eatssu/android/di/network/TokenInterceptor.kt (3 hunks)
- app/src/main/java/com/eatssu/android/ui/mypage/MyPageViewModel.kt (3 hunks)
- app/src/main/java/com/eatssu/android/ui/mypage/myreview/MyReviewAdapter.kt (3 hunks)
- app/src/main/java/com/eatssu/android/ui/mypage/myreview/MyReviewViewModel.kt (2 hunks)
- app/src/main/java/com/eatssu/android/ui/mypage/terms/WebViewActivity.kt (2 hunks)
- app/src/main/java/com/eatssu/android/ui/mypage/usernamechange/UserNameChangeViewModel.kt (2 hunks)
- app/src/main/java/com/eatssu/android/ui/review/delete/DeleteViewModel.kt (1 hunks)
- app/src/main/java/com/eatssu/android/ui/review/delete/MyReviewDialogActivity.kt (3 hunks)
- app/src/main/java/com/eatssu/android/ui/review/list/ReviewActivity.kt (5 hunks)
- app/src/main/java/com/eatssu/android/ui/review/list/ReviewViewModel.kt (1 hunks)
- app/src/main/java/com/eatssu/android/ui/review/list/ReviewViewModelFactory.kt (0 hunks)
- app/src/main/java/com/eatssu/android/ui/review/modify/ModifyReviewActivity.kt (4 hunks)
- app/src/main/java/com/eatssu/android/ui/review/modify/ModifyViewModel.kt (1 hunks)
- app/src/main/java/com/eatssu/android/ui/review/report/OthersReviewDialogActivity.kt (2 hunks)
- app/src/main/java/com/eatssu/android/ui/review/write/ImageViewModel.kt (3 hunks)
- app/src/main/java/com/eatssu/android/ui/review/write/ImageViewModelFactory.kt (1 hunks)
- app/src/main/java/com/eatssu/android/ui/review/write/ReviewWriteRateActivity.kt (15 hunks)
- app/src/main/java/com/eatssu/android/ui/review/write/ReviewWriteViewModel.kt (2 hunks)
- app/src/main/java/com/eatssu/android/ui/review/write/ReviewWriteViewModelFactory.kt (0 hunks)
- app/src/main/java/com/eatssu/android/ui/review/write/menu/ReviewWriteMenuActivity.kt (3 hunks)
- app/src/main/java/com/eatssu/android/ui/review/write/menu/VariableMenuViewModel.kt (3 hunks)
- app/src/main/res/layout/activity_my_review_dialog.xml (1 hunks)
- app/src/main/res/menu/menu_my_review.xml (1 hunks)
- app/src/main/res/values/strings.xml (1 hunks)
💤 Files with no reviewable changes (4)
- app/src/main/java/com/eatssu/android/data/repository/ImageRepository.kt
- app/src/main/java/com/eatssu/android/data/repository/ImageRepositoryImpl.kt
- app/src/main/java/com/eatssu/android/ui/review/list/ReviewViewModelFactory.kt
- app/src/main/java/com/eatssu/android/ui/review/write/ReviewWriteViewModelFactory.kt
✅ Files skipped from review due to trivial changes (2)
- app/src/main/java/com/eatssu/android/ui/review/write/ImageViewModelFactory.kt
- app/src/main/res/layout/activity_my_review_dialog.xml
🧰 Additional context used
🔇 Additional comments (51)
app/src/main/res/menu/menu_my_review.xml (1)
5-5
: 메뉴 항목 제목 변경이 적절합니다."fix"에서 "modify"로의 변경은 사용자 인터페이스의 명확성을 향상시키며, PR의 리팩토링 목표와 일치합니다. 이는 전체 애플리케이션에서 일관된 용어를 사용하는 데 도움이 됩니다.
app/src/main/java/com/eatssu/android/data/service/ImageService.kt (1)
8-8
: 💡 Codebase verificationimport 문 통합이 적절합니다.
Retrofit 어노테이션들의 import 문을 통합한 것은 코드를 더 간결하게 만듭니다.
다만, 팀의 코딩 컨벤션과 일치하는지 확인해 주세요. 다음 스크립트를 실행하여 프로젝트 내의 다른 파일들에서 import 문 스타일을 확인할 수 있습니다:
이 결과를 바탕으로 import 문 스타일의 일관성을 유지해 주세요.
✅ Verification successful
rg
명령어가 'kt' 파일 유형을 인식하지 못했습니다. 아래 스크립트를 실행하여 Kotlin 파일에서 Retrofit 어노테이션 import 문을 다시 검색해주세요:
import 문 스타일을 일관되게 유지해 주세요.
현재
ImageService.kt
에서는import retrofit2.http.*
을 사용하고 있으며, 다른 서비스 파일들에서는 개별적인 Retrofit 어노테이션을 임포트하고 있습니다. 코드베이스 전체의 일관성을 위해 임포트 스타일을 통일하는 것이 좋습니다.아래 스크립트를 실행하여 프로젝트 내의 모든 Kotlin 파일에서
import retrofit2.http.*
사용을 개별 임포트로 변경할 수 있습니다:이렇게 하면 팀의 코딩 컨벤션에 맞춰 임포트 문을 일관성 있게 유지할 수 있습니다.
import retrofit2.http.*
사용이 여전히 다른 파일들에서 발견됩니다. 아래 스크립트를 실행하여 해당 파일들을 확인해 주세요:
import 문 스타일이 일관되게 유지되고 있습니다.
프로젝트 내의 다른 Kotlin 파일에서
import retrofit2.http.*
을 사용하는 파일이 발견되지 않았습니다. 따라서 import 문 스타일이 일관성 있게 유지되고 있음을 확인했습니다.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check import statement style in other Kotlin files # Test: Search for Retrofit annotation imports in other Kotlin files rg -t kt 'import retrofit2\.http\.' --glob '!ImageService.kt'Length of output: 90
Script:
#!/bin/bash # Description: Check import statement style in other Kotlin files # Test: Search for Retrofit annotation imports in other Kotlin files rg 'import retrofit2\.http\.' --glob '*.kt' --glob '!ImageService.kt'Length of output: 1180
Script:
# #!/bin/bash # Description: Replace wildcard Retrofit imports with individual imports in Kotlin files fd --extension kt --exclude ImageService.kt | xargs sed -i 's/import retrofit2\.http\.\*/import retrofit2.http.Body\nimport retrofit2.http.POST\nimport retrofit2.http.GET/g'Length of output: 175
Script:
#!/bin/bash # Description: Find all Kotlin files using wildcard imports for Retrofit annotations rg 'import retrofit2\.http\.\*' --glob '*.kt' --glob '!ImageService.kt'Length of output: 73
app/src/main/java/com/eatssu/android/data/usecase/DeleteReviewUseCase.kt (1)
1-7
: 패키지 선언 및 임포트가 적절합니다.패키지 선언과 임포트된 클래스들이
DeleteReviewUseCase
의 기능에 적합하게 구성되어 있습니다. 불필요한 임포트가 없어 코드가 깔끔합니다.app/src/main/java/com/eatssu/android/data/usecase/WriteReviewUseCase.kt (1)
1-8
: LGTM: 패키지 선언 및 임포트가 적절합니다.패키지 선언과 임포트가 정확하고 적절합니다. 필요한 모든 클래스와 라이브러리가 임포트되어 있으며, 불필요한 임포트는 없습니다.
app/src/main/java/com/eatssu/android/data/usecase/GetMealReviewInfoUseCase.kt (1)
1-7
: LGTM: 패키지 선언 및 임포트가 적절합니다.패키지 선언과 임포트가 클래스의 기능에 맞게 잘 구성되어 있습니다. 불필요한 임포트가 없고, 필요한 모든 요소가 포함되어 있습니다.
app/src/main/java/com/eatssu/android/data/usecase/GetMenuReviewInfoUseCase.kt (1)
1-8
: 패키지 선언 및 임포트가 적절합니다.패키지 선언과 임포트가 정확하고 적절하게 이루어져 있습니다. Kotlin 코루틴의 Flow와 Hilt의 Inject를 포함하여 필요한 모든 요소가 임포트되어 있습니다.
app/src/main/java/com/eatssu/android/data/usecase/ModifyReviewUseCase.kt (3)
1-7
: LGTM: 적절한 import 문필요한 모든 클래스와 인터페이스가 올바르게 import되었습니다. 불필요한 import가 없어 코드가 깔끔합니다.
9-11
: LGTM: 적절한 클래스 선언 및 의존성 주입클래스 이름이 용도를 잘 나타내며, Hilt를 사용한 의존성 주입이 올바르게 구현되었습니다. ReviewRepository에 대한 의존성은 이 유스 케이스에 적합합니다.
12-17
: LGTM: 잘 구현된 invoke 함수suspend 키워드와 Flow의 사용이 비동기 작업에 적합합니다. operator 키워드를 통해 클래스를 함수처럼 호출할 수 있게 한 점이 좋습니다. 함수 시그니처가 repository 메서드와 일치하여 적절한 위임이 이루어지고 있습니다.
app/src/main/java/com/eatssu/android/data/usecase/GetImageUrlUseCase.kt (2)
1-9
: 패키지 선언과 임포트가 적절합니다.패키지 선언이 정확하고 필요한 모든 임포트가 포함되어 있습니다. Kotlin 코루틴과 OkHttp3 라이브러리의 사용이 적절해 보입니다.
1-17
: 전반적인 구현이 우수합니다.이 유스케이스는 Clean Architecture 원칙을 잘 따르고 있으며, Hilt를 이용한 의존성 주입과 Flow를 사용한 비동기 처리가 PR의 목표에 잘 부합합니다. 코드는 간결하고 목적에 맞게 잘 구조화되어 있습니다.
향후 개선 사항:
- 클래스와 함수에 KDoc 주석 추가
- 에러 처리 로직 고려 (현재는 repository에 위임된 것으로 보임)
- 단위 테스트 작성 검토
전체적으로 훌륭한 구현입니다. 좋은 작업입니다!
app/src/main/java/com/eatssu/android/App.kt (1)
23-25
: 로깅 설정 개선이 잘 이루어졌습니다.디버그 빌드에서만 Timber.DebugTree()를 사용하도록 조건문을 추가한 것은 좋은 변경입니다. 이는 프로덕션 환경에서 불필요한 로깅을 방지하여 성능을 향상시킬 수 있습니다.
app/src/main/java/com/eatssu/android/data/usecase/GetMenuReviewListUseCase.kt (3)
1-8
: 패키지 선언 및 임포트가 적절합니다.패키지 선언과 임포트가 클래스의 기능에 맞게 잘 구성되어 있습니다. 불필요한 임포트나 누락된 임포트가 없습니다.
1-17
: 전반적으로 잘 구현된 유스케이스입니다.이 유스케이스는 클린 아키텍처 원칙을 잘 따르고 있으며, 코틀린의 최신 기능을 적절히 활용하고 있습니다. 의존성 주입, 비동기 처리, 그리고 리포지토리 패턴의 사용이 모두 잘 이루어졌습니다.
개선을 위한 제안:
- 하드코딩된 값들을 매개변수화하거나 설정 파일에서 가져오는 것을 고려해 보세요.
null
가능성이 있는menuId
의 처리를 더 명시적으로 해주세요.- 필요하다면 에러 처리 로직을 추가하는 것을 고려해 보세요.
이러한 개선사항들을 통해 코드의 유연성과 견고성을 더욱 높일 수 있을 것입니다.
13-17
:invoke
메서드 구현이 전반적으로 잘 되었지만, 개선의 여지가 있습니다.
invoke
메서드의 구조와 비동기 처리 방식이 적절합니다. 하지만 다음 사항을 고려해 보시기 바랍니다:
MenuType.FIXED.toString()
와0
이 하드코딩되어 있습니다. 이는 유연성을 제한할 수 있습니다.menuId
가 선택적 매개변수(Long?
)인데 반해,getReviewList
메서드에 그대로 전달되고 있습니다.null
처리에 주의가 필요할 수 있습니다.다음 스크립트를 실행하여
ReviewRepository
인터페이스의getReviewList
메서드 시그니처를 확인해 주세요:이를 통해
getReviewList
메서드의 매개변수 타입과null
허용 여부를 확인할 수 있습니다.app/src/main/java/com/eatssu/android/data/usecase/GetMealReviewListUseCase.kt (2)
1-9
: 패키지 선언 및 임포트가 적절합니다.패키지 선언과 임포트된 클래스들이 사용 사례 구현에 적합하게 선택되었습니다. Kotlin 코루틴, Flow, 그리고 의존성 주입을 위한 필요한 요소들이 모두 포함되어 있습니다.
10-12
: 클래스 선언과 생성자가 PR 목표에 부합합니다.
@Inject
어노테이션을 사용한 클래스 선언과 생성자는 Hilt를 이용한 의존성 주입 패턴을 잘 따르고 있습니다. 이는 PR의 목표인 Hilt를 사용한 리팩토링에 정확히 부합합니다.ReviewRepository
를 생성자 매개변수로 받아 의존성을 외부에서 주입받는 구조가 잘 구현되었습니다.app/src/main/java/com/eatssu/android/ui/review/report/OthersReviewDialogActivity.kt (1)
7-7
: Timber 라이브러리로의 전환이 적절히 이루어졌습니다.Log에서 Timber로의 전환은 프로젝트의 로깅 메커니즘을 개선하는 좋은 변경사항입니다. Timber는 더 강력한 로깅 기능을 제공하며, 프로덕션 환경에서 더 적합합니다.
app/src/main/java/com/eatssu/android/di/DataModule.kt (1)
Line range hint
1-41
: PR 목표에 부합하는 변경 사항이 파일의 변경 사항은 PR의 목표인 ReviewService 리팩토링과 Hilt 구현에 잘 부합합니다.
ReviewRepository
에 대한 의존성 주입이 올바르게 설정되었습니다.다만,
ReviewRepositoryImpl
의 별칭 사용에 대한 이유를 명확히 하고, 코드 포맷팅을 일관되게 유지하면 코드의 가독성과 유지보수성이 더욱 향상될 것입니다.app/src/main/java/com/eatssu/android/ui/mypage/terms/WebViewActivity.kt (1)
7-7
: Timber 로깅 라이브러리로의 전환을 승인합니다.Timber 로깅 라이브러리를 도입한 것은 좋은 변경사항입니다. Timber는 Android의 기본 Log 클래스보다 더 강력한 기능을 제공하며, 로그 관리와 디버깅을 더욱 효율적으로 만들어줍니다.
이 변경은 PR의 목표인 전반적인 리팩토링 노력과 일치하며, 코드베이스의 일관성을 향상시킵니다.
app/src/main/java/com/eatssu/android/ui/mypage/myreview/MyReviewViewModel.kt (3)
18-18
: Timber 라이브러리 추가 승인Timber 라이브러리를 사용하는 것은 좋은 선택입니다. 이는 로깅을 더 효율적이고 유연하게 만들어 줍니다.
41-41
: Timber를 사용한 에러 로깅 승인Timber.e를 사용하여 에러를 로깅하는 것은 적절합니다. 이는 이전의 Log.e 사용을 대체하며, 더 나은 로깅 기능을 제공합니다.
Line range hint
1-72
: 전체적인 변경 사항 요약 및 평가이 PR에서
MyReviewViewModel.kt
에 대한 주요 변경 사항은 Timber 로깅 라이브러리의 도입입니다. 이는 다음과 같은 이점을 제공합니다:
- 더 강력하고 유연한 로깅 기능
- 코드의 일관성 향상
- 디버깅 및 문제 해결 능력 개선
전반적인 ViewModel의 구조와 기능은 그대로 유지되어, 이 변경이 기존 로직에 부정적인 영향을 미치지 않을 것으로 보입니다.
이러한 변경은 코드 품질을 향상시키는 긍정적인 리팩토링으로 보입니다. 다만, 팀 내에서 Timber 사용에 대한 가이드라인(예: 로그 레벨 사용 기준, 로그 메시지 형식 등)을 정립하고 공유하는 것이 좋겠습니다.
app/src/main/java/com/eatssu/android/ui/review/write/menu/VariableMenuViewModel.kt (1)
19-19
: Timber 임포트 추가 승인Timber 로깅 라이브러리의 임포트가 올바르게 추가되었습니다. 이는 파일 내에서 Timber를 사용하기 위해 필요한 변경사항입니다.
app/src/main/java/com/eatssu/android/ui/review/write/menu/ReviewWriteMenuActivity.kt (1)
17-17
: Timber 로깅 라이브러리로의 전환을 승인합니다.Timber 로깅 라이브러리를 도입한 것은 좋은 변경사항입니다. Timber는 자동 태깅, 크래시 방지 로깅 등 Android의 기본 Log 유틸리티보다 많은 이점을 제공합니다. 이는 디버깅과 유지보수를 더욱 효율적으로 만들 것입니다.
app/src/main/java/com/eatssu/android/ui/mypage/myreview/MyReviewAdapter.kt (1)
15-15
: Timber 라이브러리 도입 승인Timber 라이브러리로의 전환이 올바르게 이루어졌습니다. 이는 로깅 기능을 개선하고 일관성을 유지하는 데 도움이 될 것입니다.
app/src/main/java/com/eatssu/android/ui/mypage/usernamechange/UserNameChangeViewModel.kt (1)
19-19
: Timber 라이브러리 도입 승인Timber 라이브러리의 도입은 애플리케이션 전반의 로깅 메커니즘을 개선하는 데 도움이 될 것입니다. 이는 코드베이스의 다른 부분에서 이루어진 변경사항과 일치하며, 로깅 관행을 향상시키는 좋은 방법입니다.
app/src/main/java/com/eatssu/android/ui/mypage/MyPageViewModel.kt (4)
25-25
: Timber 라이브러리 추가 승인Timber 라이브러리를 import하는 것은 로깅 방식을 개선하는 좋은 접근입니다. 이는 더 효율적이고 유연한 로깅을 가능하게 합니다.
73-75
: getMyInfo() 메서드의 로깅 개선 승인Android Log에서 Timber로의 전환은 적절합니다. 이는 코드의 가독성을 향상시키고 더 강력한 로깅 기능을 제공합니다. 기존 로직은 변경되지 않았으며, 로깅 방식만 개선되었습니다.
124-126
: signOut() 메서드의 로깅 개선 승인signOut() 메서드에서도 Android Log에서 Timber로의 전환이 일관되게 적용되었습니다. 이는 전체적인 로깅 방식의 통일성을 유지하며, 코드의 품질을 향상시킵니다.
Line range hint
1-186
: 전체 파일 리뷰 요약MyPageViewModel.kt 파일의 변경사항은 주로 로깅 방식의 개선에 초점을 맞추고 있습니다. Android Log에서 Timber로의 전환은 일관되게 적용되었으며, 이는 코드의 가독성과 유지보수성을 향상시킵니다.
주요 개선 사항:
- Timber 라이브러리 도입
- 모든 로그 호출을 Timber로 대체
- 기존 로직 및 구조 유지
이러한 변경은 PR의 목표인 리팩토링과 코드베이스 개선에 부합합니다. ViewModel의 기능은 그대로 유지되면서 로깅 품질이 향상되었습니다.
app/src/main/res/values/strings.xml (1)
74-74
: 문자열 리소스 수정 승인"수정하기"로 변경된 것이 적절해 보입니다. 이는 사용자 인터페이스의 일관성과 명확성을 개선합니다.
app/src/main/java/com/eatssu/android/data/service/ReviewService.kt (7)
10-11
: **의존성 추가 확인 **
ImageResponse
와MultipartBody
에 대한 import가 추가되었습니다. 이는 이미지 업로드 기능 구현에 필요한 적절한 추가입니다.
17-20
: **코루틴 지원을 위한suspend
키워드 사용 **
writeReview
메서드에suspend
키워드가 추가되어 코루틴을 지원합니다. Retrofit과의 비동기 통신을 위해 적절한 변경입니다.
23-25
: **코루틴 지원을 위한suspend
키워드 사용 **
deleteReview
메서드에suspend
키워드가 추가되었습니다. 비동기 처리를 위한 올바른 수정입니다.
28-31
: **코루틴 지원을 위한suspend
키워드 사용 **
modifyReview
메서드에suspend
키워드가 추가되었습니다. 적절한 비동기 지원 수정입니다.
35-44
: **페이징 기능 추가 확인 **
getReviewList
메서드에 페이징을 위한page
,size
,sort
파라미터가 추가되었습니다. 기본값도 설정되어 있어 활용하기 편리합니다.
46-48
: **코루틴 지원을 위한suspend
키워드 사용 **
getMenuReviewInfo
메서드에suspend
키워드가 추가되었습니다. 비동기 처리를 위한 적절한 변경입니다.
51-53
: **코루틴 지원을 위한suspend
키워드 사용 **
getMealReviewInfo
메서드에suspend
키워드가 추가되어 비동기 지원이 강화되었습니다.app/src/main/java/com/eatssu/android/ui/review/modify/ModifyViewModel.kt (2)
22-25
:@HiltViewModel
주석 및 의존성 주입 확인
ModifyViewModel
클래스에@HiltViewModel
주석이 추가되고,ModifyReviewUseCase
를 주입받고 있습니다. 의존성 주입이 올바르게 설정되었는지 확인해주세요.
50-57
: 상태 업데이트 로직 일관성 확인
collectLatest
블록에서 성공적으로 데이터를 수신하면_uiState
를 업데이트하면서error = false
,isDone = true
로 설정하고 있습니다. 이는 적절하지만, 전체적인 상태 관리 로직이 일관성 있는지 다시 한번 확인해주세요.현재 상태 관리 로직을 확인하기 위해 다음 스크립트를 실행해볼 것을 제안합니다:
✅ Verification successful
상태 업데이트 로직 일관성 확인 완료
ModifyViewModel.kt
에서_uiState
의 상태 업데이트 로직이 전체 코드베이스의 다른 ViewModel들과 일관되게 구현되어 있습니다.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: ModifyViewModel에서 _uiState의 error와 isDone 상태 업데이트 로직 확인 # Test: error와 isDone 업데이트 부분 검색 rg '_uiState\.update.*\b(error|isDone)\b'Length of output: 4359
Script:
#!/bin/bash # Description: 모든 ViewModel에서 _uiState의 isDone 상태 업데이트 로직 확인 # Test: isDone 업데이트 부분 검색 rg '_uiState\.update.*\bisDone\b'Length of output: 380
app/src/main/java/com/eatssu/android/ui/review/modify/ModifyReviewActivity.kt (4)
15-15
: @androidentrypoint 어노테이션 적용이 적절합니다.Hilt를 사용한 의존성 주입을 위해
@AndroidEntryPoint
어노테이션을 추가한 것은 좋은 선택입니다.
18-18
: ViewModel 초기화를 간결하게 개선하였습니다.
by viewModels()
위임 프로퍼티를 사용하여 ViewModel을 간단하게 초기화한 것은 코드의 가독성과 유지보수성을 향상시킵니다.
74-77
: ModifyReviewRequest 객체 사용으로 데이터 전달을 명확하게 개선했습니다.개별 매개변수 대신
ModifyReviewRequest
객체를 전달함으로써 코드의 가독성과 데이터 구조의 일관성이 향상되었습니다.
83-86
: ViewModel의 uiState를 효과적으로 관찰하고 있습니다.작업 완료 시 토스트 메시지를 표시하고 액티비티를 종료하는 로직이 올바르게 구현되었습니다.
app/src/main/java/com/eatssu/android/di/network/TokenInterceptor.kt (2)
40-41
:TAG
상수 위치 변경 확인했습니다.
TAG
상수를 companion object 내로 이동하여 코드 구조가 개선되었습니다.
67-68
: HTTP 헤더 추가 부분 확인 및 수정 사항 고려헤더를 추가한 부분은 적절해 보입니다. 다만, 앞서 지적한
HEADER_ACCEPT
의 대소문자 문제가 수정되면 해당 부분도 자동으로 수정됩니다.app/src/main/java/com/eatssu/android/ui/review/write/ReviewWriteRateActivity.kt (4)
15-15
: 의존성 주입을 위한 Hilt 적용 확인
@AndroidEntryPoint
어노테이션과by viewModels()
를 사용하여 Hilt를 통한 의존성 주입이 올바르게 적용되었습니다. 잘 수행하셨습니다.Also applies to: 26-26, 36-36
30-30
: 로그 출력 시 Timber 사용으로 변경기존의
Log.d
대신Timber
를 사용하여 로그를 출력하도록 변경하였습니다. 로그 관리가 더욱 편리해졌습니다.Also applies to: 63-63, 89-89, 111-111, 129-129, 150-150, 208-208, 229-229, 255-255, 312-312, 342-342, 352-352
40-41
: ViewModel 주입 방식 업데이트 확인
uploadReviewViewModel
과imageviewModel
을lateinit
에서by viewModels()
로 변경하여 Hilt를 통한 ViewModel 주입을 사용하였습니다. 올바르게 적용되었습니다.
314-314
: 사용자에게 표시되는 메시지의 일관성 확인이미지 삭제 시 사용자에게 보여주는 메시지가 명확하고 일관성 있게 전달되고 있는지 확인해주세요.
Also applies to: 325-325
app/src/main/java/com/eatssu/android/data/service/ImageService.kt
Outdated
Show resolved
Hide resolved
suspend operator fun invoke( | ||
mealId: Long?, | ||
): Flow<BaseResponse<GetReviewListResponse>> = | ||
reviewRepository.getReviewList(MenuType.VARIABLE.toString(), mealId, 0) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
invoke 함수 구현에 대한 개선 제안
suspend operator 함수로 구현된 invoke
메서드는 코루틴과 Flow를 활용하여 PR 목표에 잘 부합합니다. 그러나 다음 사항들을 고려해 보시기 바랍니다:
reviewRepository.getReviewList
호출 시 사용된 하드코딩된 값0
에 대한 설명이나 리팩토링이 필요해 보입니다.- 이 use case 내에서 명시적인 에러 처리가 구현되어 있지 않습니다. 에러 처리 방식에 대해 고려해 보시기 바랍니다.
MenuType.VARIABLE.toString()
의 사용은 타입 안정성을 위해 개선될 수 있습니다.
다음과 같은 개선을 제안합니다:
class GetMealReviewListUseCase @Inject constructor(
private val reviewRepository: ReviewRepository,
) {
suspend operator fun invoke(
mealId: Long?,
menuType: MenuType = MenuType.VARIABLE,
menuId: Long? = null
): Flow<BaseResponse<GetReviewListResponse>> =
reviewRepository.getReviewList(menuType.name, mealId, menuId)
}
이렇게 수정하면 함수의 유연성이 향상되고, 하드코딩된 값들을 제거할 수 있습니다. 또한, MenuType
을 직접 전달하여 타입 안정성을 개선할 수 있습니다.
app/src/main/java/com/eatssu/android/ui/mypage/myreview/MyReviewAdapter.kt
Outdated
Show resolved
Hide resolved
@@ -37,7 +38,7 @@ class UserNameChangeViewModel @Inject constructor( | |||
_uiState.update { it.copy(loading = false, error = true) } | |||
}.catch { e -> | |||
_uiState.update { it.copy(error = true, toastMessage = "닉네임 중복 확인에 실패했습니다.") } | |||
Log.e(TAG, e.toString()) | |||
Timber.e(e.toString()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Timber 사용 승인 및 오류 처리 로직 개선 제안
Timber를 사용한 로깅으로의 변경은 적절합니다. 그러나 onCompletion
블록에서 error
상태를 항상 true
로 설정하는 것은 문제가 될 수 있습니다. 이는 작업이 성공적으로 완료된 경우에도 오류 상태로 처리될 수 있기 때문입니다.
다음과 같이 수정하는 것이 좋습니다:
.onCompletion { error ->
_uiState.update { it.copy(loading = false, error = error != null) }
}
이렇게 하면 실제 오류가 발생한 경우에만 error
상태가 true
로 설정됩니다.
suspend fun writeReview( | ||
menuId: Long, | ||
body: WriteReviewRequest, | ||
): Flow<BaseResponse<Void>> | ||
|
||
// suspend fun getMenuReviewInfo(menuId: Long) | ||
// : Flow<BaseResponse<GetMenuReviewInfoResponse>> = | ||
// flow { | ||
// emit(reviewService.getMenuReviewInfo(menuId)) | ||
// } | ||
suspend fun deleteReview( | ||
reviewId: Long, | ||
): Flow<BaseResponse<Void>> | ||
|
||
suspend fun modifyReview( | ||
reviewId: Long, | ||
body: ModifyReviewRequest, | ||
): Flow<BaseResponse<Void>> | ||
|
||
suspend fun getReviewList( | ||
menuType: String, | ||
mealId: Long?, | ||
menuId: Long?, | ||
): Flow<BaseResponse<GetReviewListResponse>> | ||
|
||
suspend fun getMenuReviewInfo( | ||
menuId: Long, | ||
): Flow<BaseResponse<GetMenuReviewInfoResponse>> | ||
|
||
|
||
suspend fun getMealReviewInfo( | ||
mealId: Long, | ||
): Flow<BaseResponse<GetMealReviewInfoResponse>> | ||
|
||
suspend fun getImageString( | ||
image: MultipartBody.Part, | ||
): Flow<BaseResponse<ImageResponse>> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
'suspend' 키워드의 불필요한 사용 검토
'Flow'를 반환하는 함수에서는 일반적으로 'suspend' 키워드가 필요하지 않습니다. 'Flow'는 자체적으로 비동기 스트림을 나타내며, 수집할 때 코루틴 내에서 처리되므로 인터페이스 메서드에서 'suspend'를 제거하여 코드의 간결성과 명확성을 높일 수 있습니다.
적용할 변경 사항:
- suspend fun writeReview(
+ fun writeReview(
- suspend fun deleteReview(
+ fun deleteReview(
- suspend fun modifyReview(
+ fun modifyReview(
- suspend fun getReviewList(
+ fun getReviewList(
- suspend fun getMenuReviewInfo(
+ fun getMenuReviewInfo(
- suspend fun getMealReviewInfo(
+ fun getMealReviewInfo(
- suspend fun getImageString(
+ fun getImageString(
📝 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.
suspend fun writeReview( | |
menuId: Long, | |
body: WriteReviewRequest, | |
): Flow<BaseResponse<Void>> | |
// suspend fun getMenuReviewInfo(menuId: Long) | |
// : Flow<BaseResponse<GetMenuReviewInfoResponse>> = | |
// flow { | |
// emit(reviewService.getMenuReviewInfo(menuId)) | |
// } | |
suspend fun deleteReview( | |
reviewId: Long, | |
): Flow<BaseResponse<Void>> | |
suspend fun modifyReview( | |
reviewId: Long, | |
body: ModifyReviewRequest, | |
): Flow<BaseResponse<Void>> | |
suspend fun getReviewList( | |
menuType: String, | |
mealId: Long?, | |
menuId: Long?, | |
): Flow<BaseResponse<GetReviewListResponse>> | |
suspend fun getMenuReviewInfo( | |
menuId: Long, | |
): Flow<BaseResponse<GetMenuReviewInfoResponse>> | |
suspend fun getMealReviewInfo( | |
mealId: Long, | |
): Flow<BaseResponse<GetMealReviewInfoResponse>> | |
suspend fun getImageString( | |
image: MultipartBody.Part, | |
): Flow<BaseResponse<ImageResponse>> | |
fun writeReview( | |
menuId: Long, | |
body: WriteReviewRequest, | |
): Flow<BaseResponse<Void>> | |
fun deleteReview( | |
reviewId: Long, | |
): Flow<BaseResponse<Void>> | |
fun modifyReview( | |
reviewId: Long, | |
body: ModifyReviewRequest, | |
): Flow<BaseResponse<Void>> | |
fun getReviewList( | |
menuType: String, | |
mealId: Long?, | |
menuId: Long?, | |
): Flow<BaseResponse<GetReviewListResponse>> | |
fun getMenuReviewInfo( | |
menuId: Long, | |
): Flow<BaseResponse<GetMenuReviewInfoResponse>> | |
fun getMealReviewInfo( | |
mealId: Long, | |
): Flow<BaseResponse<GetMealReviewInfoResponse>> | |
fun getImageString( | |
image: MultipartBody.Part, | |
): Flow<BaseResponse<ImageResponse>> |
_uiState.update { it.copy(loading = false, error = true) } | ||
}.catch { e -> | ||
_uiState.update { | ||
it.copy( | ||
loading = false, | ||
error = false, | ||
) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
에러 상태 업데이트 로직 수정 필요
callMenuReviewList
메서드의 에러 상태 업데이트 로직도 수정이 필요합니다. onCompletion
에서 error
를 true
로 설정하고, catch
에서 error
를 false
로 설정하고 있습니다. 일관된 에러 처리를 위해 로직을 수정하세요.
수정 제안:
.onCompletion {
- _uiState.update { it.copy(loading = false, error = true) }
+ _uiState.update { it.copy(loading = false, error = false) }
}.catch { e ->
_uiState.update {
it.copy(
loading = false,
- error = false,
+ error = true,
)
}
Timber.e(e)
}
📝 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.
_uiState.update { it.copy(loading = false, error = true) } | |
}.catch { e -> | |
_uiState.update { | |
it.copy( | |
loading = false, | |
error = false, | |
) | |
} | |
_uiState.update { it.copy(loading = false, error = false) } | |
}.catch { e -> | |
_uiState.update { | |
it.copy( | |
loading = false, | |
error = true, | |
) | |
} | |
Timber.e(e) |
@@ -233,7 +226,7 @@ class ReviewWriteRateActivity : | |||
Manifest.permission.READ_MEDIA_IMAGES, | |||
), REQ_GALLERY | |||
) | |||
Log.d("ReviewWriteRateActivity", "권한 없음") | |||
Timber.e("권한 없음") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
권한 요청 로직에서 오류 발생 가능성
checkPermission()
메서드에서 안드로이드 버전에 따른 권한 요청 처리 중 Timber.e("권한 없음")
이 두 번 사용되었습니다. 하지만 else
블록 내부에 있어 권한이 있는 경우에도 로그가 출력될 수 있습니다.
Timber.e("권한 없음")
로그를 권한이 거부된 경우에만 출력하도록 수정해주세요.
Also applies to: 255-255
@@ -280,11 +273,11 @@ | |||
} | |||
|
|||
private fun requestStoragePermission() { | |||
if (checkSelfPermission(android.Manifest.permission.WRITE_EXTERNAL_STORAGE) != | |||
if (checkSelfPermission(Manifest.permission.WRITE_EXTERNAL_STORAGE) != |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
권한 요청 메서드 업데이트 필요
requestStoragePermission()
메서드에서 WRITE_EXTERNAL_STORAGE
권한을 요청하고 있습니다. 안드로이드 13(API 33)부터는 해당 권한이 더 이상 필요하지 않습니다. 대신 READ_MEDIA_IMAGES
권한을 요청하도록 업데이트해주세요.
다음과 같이 코드를 수정하는 것을 권장합니다:
- if (checkSelfPermission(Manifest.permission.WRITE_EXTERNAL_STORAGE) !=
+ if (checkSelfPermission(Manifest.permission.READ_MEDIA_IMAGES) !=
PackageManager.PERMISSION_GRANTED
) {
requestPermissions(
- arrayOf(Manifest.permission.WRITE_EXTERNAL_STORAGE),
+ arrayOf(Manifest.permission.READ_MEDIA_IMAGES),
PERMISSION_REQUEST_CODE
)
}
Also applies to: 280-280
e0b0bdc
to
e589b47
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 14
🧹 Outside diff range and nitpick comments (52)
app/src/main/java/com/eatssu/android/data/usecase/auth/GetAccessTokenUseCase.kt (2)
Line range hint
1-13
: 아키텍처 개선 제안UseCase가 데이터 계층의 구현 세부사항(MySharedPreferences)에 직접 의존하고 있습니다. Clean Architecture 원칙에 따라 Repository 패턴을 통해 이 의존성을 추상화하는 것이 좋습니다.
다음과 같은 리팩토링을 제안합니다:
+interface AuthRepository { + suspend fun getAccessToken(): String +} +class AuthRepositoryImpl @Inject constructor( + @ApplicationContext private val context: Context +) : AuthRepository { + override suspend fun getAccessToken(): String = + MySharedPreferences.getAccessToken(context) +} class GetAccessTokenUseCase @Inject constructor( - @ApplicationContext private val context: Context + private val authRepository: AuthRepository ) { suspend operator fun invoke(): String { - return MySharedPreferences.getAccessToken(context) + return authRepository.getAccessToken() } }
보안 취약점 발견: 토큰 저장소 암호화 필요
현재 구현에서 다음과 같은 보안 취약점이 확인되었습니다:
- 액세스 토큰과 리프레시 토큰이 일반 SharedPreferences에 평문으로 저장됨
- 암호화나 보안 메커니즘이 전혀 적용되지 않음
필수 개선사항:
EncryptedSharedPreferences
사용으로 전환- 토큰 저장 시 Android Keystore System을 활용한 암호화 적용
editor.commit()
대신editor.apply()
로 통일 (이미 일부 메서드에서 사용 중)🔗 Analysis chain
Line range hint
1-13
: 보안 고려사항 검토 필요액세스 토큰을 SharedPreferences에 저장하는 것은 보안상 취약할 수 있습니다. 더 안전한 저장소 메커니즘을 고려해 보시기 바랍니다.
다음과 같은 개선사항을 고려해보세요:
- EncryptedSharedPreferences 사용
- Android Keystore System 활용
- 토큰 암호화 적용
토큰 저장 구현을 확인하기 위해 다음 스크립트를 실행합니다:
Let me check if there are any encryption-related implementations in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check the implementation of token storage in MySharedPreferences # Check if encryption is used for token storage rg -A 5 "getAccessToken|setAccessToken" app/src/main/java/com/eatssu/android/util/Length of output: 3172
Script:
#!/bin/bash # Check for any encryption-related implementations rg -i "encrypt|cipher|keystore" app/src/main/java/com/eatssu/android/ # Check the complete implementation of MySharedPreferences cat app/src/main/java/com/eatssu/android/util/MySharedPreferences.ktLength of output: 3868
app/src/main/java/com/eatssu/android/data/usecase/review/DeleteReviewUseCase.kt (1)
11-13
: 에러 처리 개선을 고려해보세요.현재 구현은 repository의 에러 처리에만 의존하고 있습니다. UseCase 레벨에서 다음과 같은 추가적인 에러 처리를 고려해보시는 것이 좋을 것 같습니다:
- reviewId 유효성 검증
- 비즈니스 로직 관련 예외 처리
다음과 같은 개선된 구현을 제안드립니다:
suspend operator fun invoke(reviewId: Long): Flow<BaseResponse<Void>> = - reviewRepository.deleteReview(reviewId) + try { + require(reviewId > 0) { "리뷰 ID는 0보다 커야 합니다" } + reviewRepository.deleteReview(reviewId) + } catch (e: Exception) { + flow { + emit(BaseResponse.Error(e.message ?: "리뷰 삭제 중 오류가 발생했습니다")) + } + }app/src/main/java/com/eatssu/android/data/usecase/auth/GetUserNameUseCase.kt (2)
Line range hint
8-11
: 주석 처리된 코드 제거 필요주석 처리된 코드는 버전 관리 시스템에서 이력을 확인할 수 있으므로 제거하는 것이 좋습니다.
class GetUserNameUseCase @Inject constructor( - // private val preferencesRepository: PreferencesRepository, @ApplicationContext private val context: Context ) {
Line range hint
8-14
: 아키텍처 개선 제안현재 구현에서 발견된 문제점들:
MySharedPreferences
를 직접 사용하는 것은 Clean Architecture의 원칙에 위배됩니다.- Repository 패턴이 제거되어 데이터 접근 계층이 UseCase에 직접 노출되어 있습니다.
- 동기 작업인 SharedPreferences 접근에 suspend 함수를 사용하는 것은 불필요할 수 있습니다.
다음과 같은 개선을 제안드립니다:
// PreferencesRepository.kt interface PreferencesRepository { fun getUserName(): String } // PreferencesRepositoryImpl.kt class PreferencesRepositoryImpl @Inject constructor( @ApplicationContext private val context: Context ) : PreferencesRepository { override fun getUserName(): String = MySharedPreferences.getUserName(context) } // GetUserNameUseCase.kt class GetUserNameUseCase @Inject constructor( private val preferencesRepository: PreferencesRepository ) { operator fun invoke(): String = preferencesRepository.getUserName() }app/src/main/java/com/eatssu/android/data/usecase/auth/GetRefreshTokenUseCase.kt (1)
Line range hint
8-11
: Repository 패턴 적용을 고려해주세요.
MySharedPreferences
를 직접 사용하는 대신, 주석 처리된PreferencesRepository
를 활용하는 것이 더 나은 아키텍처가 될 것 같습니다. Repository 패턴을 통해:
- 데이터 소스 추상화
- 테스트 용이성 향상
- 의존성 역전 원칙 준수
가 가능해집니다.다음과 같은 리팩토링을 제안드립니다:
class GetRefreshTokenUseCase @Inject constructor( @ApplicationContext private val context: Context -// private val preferencesRepository: PreferencesRepository, + private val preferencesRepository: PreferencesRepository, ) { suspend operator fun invoke(): String { - return MySharedPreferences.getRefreshToken(context) + return preferencesRepository.getRefreshToken() } }app/src/main/java/com/eatssu/android/data/usecase/auth/SetAccessTokenUseCase.kt (3)
Line range hint
8-11
: 주석 처리된 코드를 정리해주세요주석 처리된
preferencesRepository
매개변수가 있습니다. 이 코드가 더 이상 필요하지 않다면 완전히 제거하는 것이 좋습니다.class SetAccessTokenUseCase @Inject constructor( - // private val preferencesRepository: PreferencesRepository, @ApplicationContext private val context: Context ) {
Line range hint
12-14
: 아키텍처 개선 제안: Repository 패턴 적용 검토현재 구현에서는
MySharedPreferences
를 직접 사용하고 있습니다. Clean Architecture 원칙에 따라 다음 사항들을 고려해보시기 바랍니다:
PreferencesRepository
인터페이스를 통한 추상화 계층 도입- 테스트 용이성 향상을 위한 의존성 분리
- 데이터 접근 계층의 일관성 유지
예시 구현:
interface PreferencesRepository { suspend fun setAccessToken(token: String) } class PreferencesRepositoryImpl @Inject constructor( @ApplicationContext private val context: Context ) : PreferencesRepository { override suspend fun setAccessToken(token: String) { MySharedPreferences.setAccessToken(context, token) } } class SetAccessTokenUseCase @Inject constructor( private val preferencesRepository: PreferencesRepository ) { suspend operator fun invoke(accessToken: String) { preferencesRepository.setAccessToken(accessToken) } }
SharedPreferences의 동시성 처리가 불완전합니다
MySharedPreferences
구현을 검토한 결과, 다음과 같은 동시성 관련 문제점들이 발견되었습니다:
- 일부 메서드에서는
commit()
을, 다른 메서드에서는apply()
를 혼용하고 있어 일관성이 없습니다.object
싱글톤으로 구현되어 있지만, 각 메서드에서 매번 새로운SharedPreferences
인스턴스와Editor
를 생성하고 있습니다.- 토큰 저장/조회 시 동기화 메커니즘이 없어 race condition이 발생할 수 있습니다.
개선 권장사항:
- 모든 쓰기 작업에
apply()
를 일관되게 사용하거나, 즉시 반영이 필요한 경우commit()
으로 통일SharedPreferences
인스턴스를 싱글톤으로 캐싱하여 재사용- 토큰 관련 작업에
@Synchronized
어노테이션 추가 고려🔗 Analysis chain
Line range hint
1-14
: 동시성 처리 검증 필요
MySharedPreferences
를 사용한 토큰 저장 시 동시성 문제가 발생할 수 있습니다. 여러 코루틴에서 동시에 접근할 때의 안전성을 검증해주세요.
Let me analyze the code further to understand the implementation details of
MySharedPreferences
.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: SharedPreferences 관련 동시성 처리 코드 확인 # SharedPreferences 사용 패턴 검색 rg -A 3 "SharedPreferences\." # commit/apply 사용 패턴 검색 rg -A 3 "\.(commit|apply)\(\)"Length of output: 11216
Script:
#!/bin/bash # Check MySharedPreferences implementation details rg -l "class MySharedPreferences" | xargs cat # Check if there's any synchronization mechanism rg "@Synchronized|synchronized|Mutex|@Volatile|volatile"Length of output: 106
Script:
#!/bin/bash # MySharedPreferences 구현 확인을 위한 전체 파일 내용 조회 cat app/src/main/java/com/eatssu/android/util/MySharedPreferences.kt # companion object 내부 구현 확인 rg -A 10 "companion object" app/src/main/java/com/eatssu/android/util/MySharedPreferences.ktLength of output: 3890
app/src/main/java/com/eatssu/android/data/usecase/auth/SetRefreshTokenUseCase.kt (2)
Line range hint
8-10
: 주석 처리된 코드를 제거해주세요.주석 처리된
preferencesRepository
매개변수가 더 이상 필요하지 않다면 완전히 제거하는 것이 좋습니다. 주석으로 남겨두면 코드의 가독성이 떨어지고 혼란을 줄 수 있습니다.class SetRefreshTokenUseCase @Inject constructor( - // private val preferencesRepository: PreferencesRepository, @ApplicationContext private val context: Context )
SharedPreferences 관련 UseCase들의 아키텍처 개선이 필요합니다
코드베이스 분석 결과, 다음과 같은 아키텍처 문제점들이 확인되었습니다:
MySharedPreferences
는 유틸리티 클래스로 구현되어 있으며, 여러 UseCase들이 이를 직접 참조하고 있습니다.DataModule
에서 Repository 패턴을 사용하고 있지만, SharedPreferences 관련 데이터 접근은 Repository 패턴을 따르지 않고 있습니다.- 현재 구조는 Clean Architecture의 계층 분리 원칙을 위반하고 있습니다.
개선 제안:
- SharedPreferences 관련 로직을 Repository 계층으로 이동
- UseCase는 Repository 인터페이스에만 의존하도록 수정
MySharedPreferences
구현을 Repository 구현체로 캡슐화이는 테스트 용이성과 유지보수성 향상을 위해 필요한 변경입니다.
🔗 Analysis chain
Line range hint
1-14
: Clean Architecture 원칙 검토가 필요합니다.현재 구현에서 몇 가지 아키텍처적 고려사항이 있습니다:
- UseCase가 직접
MySharedPreferences
를 호출하는 것은 Clean Architecture의 계층 분리 원칙에 위배될 수 있습니다.- 이전에 제거된
PreferencesRepository
가 이러한 세부 구현을 추상화하는 역할을 했을 것으로 보입니다.데이터 계층의 세부 구현을 UseCase에서 직접 다루는 대신, Repository 패턴을 통해 추상화하는 것을 고려해주세요.
Let me search for more information about the architecture and SharedPreferences usage.
Let me check the PreferencesRepository implementation and its relationship with SharedPreferences.
Let me try one more search with different patterns to understand the SharedPreferences usage pattern.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Repository 패턴 사용 여부 확인 # PreferencesRepository 인터페이스 존재 여부 확인 echo "Checking for PreferencesRepository interface..." rg -l "interface.*PreferencesRepository" # SharedPreferences 관련 Repository 구현체 확인 echo "Checking for SharedPreferences repository implementations..." rg -l "class.*Repository.*SharedPreferences"Length of output: 325
Script:
#!/bin/bash # Check for any SharedPreferences related files and usages echo "Checking SharedPreferences related files..." rg -l "SharedPreferences" # Check for Repository pattern usage in the project echo "Checking Repository pattern usage..." rg -l "Repository" # Check for data layer structure echo "Checking data layer structure..." fd . "app/src/main/java/com/eatssu/android/data" # Check MySharedPreferences implementation echo "Checking MySharedPreferences implementation..." rg -l "MySharedPreferences"Length of output: 11741
Script:
#!/bin/bash # Check PreferencesRepository implementation echo "Checking PreferencesRepository content..." rg -A 10 "interface PreferencesRepository" app/src/main/java/com/eatssu/android/data/repository/PreferencesRepository.kt # Check if there are any implementations of PreferencesRepository echo "Checking PreferencesRepository implementations..." ast-grep --pattern 'class $_ implements PreferencesRepository' # Check MySharedPreferences implementation details echo "Checking MySharedPreferences implementation..." rg -A 10 "class MySharedPreferences" app/src/main/java/com/eatssu/android/util/MySharedPreferences.ktLength of output: 596
Script:
#!/bin/bash # Check MySharedPreferences content with different pattern echo "Checking MySharedPreferences content..." rg -A 20 "object MySharedPreferences" app/src/main/java/com/eatssu/android/util/MySharedPreferences.kt # Check other auth related UseCases echo "Checking auth related UseCases..." rg -l "class.*UseCase.*context: Context" app/src/main/java/com/eatssu/android/data/usecase/auth/ # Check DataModule for DI setup echo "Checking DataModule..." rg -A 20 "class DataModule" app/src/main/java/com/eatssu/android/di/DataModule.ktLength of output: 1916
app/src/main/java/com/eatssu/android/data/repository/MealRepository.kt (2)
8-12
: 주석 처리된 코드 정리 필요주석 처리된
getTodayMeal
메서드가 있습니다. 이 메서드가 향후 구현될 예정이라면 TODO 주석을 추가하고, 그렇지 않다면 제거하는 것이 좋습니다.-// suspend fun getTodayMeal( -// date: String, -// restaurant: String, -// time: String, -// ): Flow<BaseResponse<ArrayList<GetMealResponse>>> + // TODO: getTodayMeal 메서드 구현 예정 + // suspend fun getTodayMeal( + // date: String, + // restaurant: String, + // time: String, + // ): Flow<BaseResponse<ArrayList<GetMealResponse>>>
7-17
: 리포지토리 인터페이스 설계가 잘 되었습니다Clean Architecture와 MVVM 패턴에 맞게 잘 설계되었습니다:
- Flow를 사용한 반응형 프로그래밍 적용
- suspend 함수를 통한 비동기 처리
- 명확한 메서드 시그니처
다만, 다음 사항들을 검토해 주시면 좋겠습니다:
- 메서드에 대한 KDoc 문서화 추가
- 예외 처리 방식에 대한 명세 추가
interface MealRepository { + /** + * 주어진 식사 ID에 해당하는 메뉴 정보를 조회합니다. + * + * @param mealId 조회할 식사의 ID + * @return 메뉴 정보를 포함한 응답 + * @throws IOException 네트워크 오류 발생 시 + */ suspend fun getMenuInfoByMealId( mealId: Long, ): Flow<BaseResponse<MenuOfMealResponse>>app/src/main/java/com/eatssu/android/data/usecase/menu/GetMenuNameListOfMealUseCase.kt (1)
9-11
: 의존성 주입이 잘 구현되어 있습니다만, 파라미터 이름을 검토해주세요.클래스 구조와 Hilt를 사용한 의존성 주입이 잘 구현되어 있습니다. 다만, 아래 사항을 고려해주세요:
invoke
메서드의 파라미터 이름menuId
와 repository 메서드의 파라미터 이름mealId
가 일치하지 않습니다. 일관성을 위해 통일하는 것이 좋겠습니다.- suspend operator fun invoke(menuId: Long): Flow<BaseResponse<MenuOfMealResponse>> = - mealRepository.getMenuInfoByMealId(menuId) + suspend operator fun invoke(mealId: Long): Flow<BaseResponse<MenuOfMealResponse>> = + mealRepository.getMenuInfoByMealId(mealId)app/src/main/java/com/eatssu/android/data/usecase/review/WriteReviewUseCase.kt (1)
12-14
: Flow 반환 타입 사용이 적절합니다만, 예외 처리를 고려해보세요.suspend 함수와 Flow를 사용하여 비동기 처리를 깔끔하게 구현했습니다. 하지만 네트워크 오류나 기타 예외 상황에 대한 처리가 repository에만 의존하고 있습니다.
다음과 같이 예외 처리를 추가하는 것을 고려해보세요:
suspend operator fun invoke(menuId: Long, body: WriteReviewRequest): Flow<BaseResponse<Void>> = - reviewRepository.writeReview(menuId, body) + try { + reviewRepository.writeReview(menuId, body) + } catch (e: Exception) { + Timber.e(e, "리뷰 작성 중 오류 발생") + throw e + }app/src/main/java/com/eatssu/android/data/usecase/review/GetMealReviewInfoUseCase.kt (1)
9-11
: 클래스 구조가 잘 설계되었습니다!Hilt를 사용한 의존성 주입이 잘 구현되어 있습니다. 단, 추후 유지보수를 위해 클래스에 대한 KDoc 문서화를 추가하면 좋을 것 같습니다.
다음과 같이 문서화를 추가해보세요:
+/** + * 식사 리뷰 정보를 조회하는 UseCase + * + * @property reviewRepository 리뷰 저장소 인터페이스 + */ class GetMealReviewInfoUseCase @Inject constructor( private val reviewRepository: ReviewRepository, )app/src/main/java/com/eatssu/android/data/usecase/review/GetMenuReviewInfoUseCase.kt (1)
12-14
: Flow 사용이 적절합니다만, 예외 처리를 고려해보세요.Flow를 사용한 비동기 처리가 적절하게 구현되었습니다. 하지만 네트워크 오류나 기타 예외 상황에 대한 처리가 보이지 않습니다.
다음과 같은 예외 처리 로직 추가를 고려해보세요:
suspend operator fun invoke(menuId: Long): Flow<BaseResponse<GetMenuReviewInfoResponse>> = - reviewRepository.getMenuReviewInfo(menuId) + reviewRepository.getMenuReviewInfo(menuId) + .catch { exception -> + emit(BaseResponse.Error(exception)) + }app/src/main/java/com/eatssu/android/data/usecase/review/ModifyReviewUseCase.kt (1)
12-17
: 에러 처리에 대한 문서화가 필요합니다.함수 구현은 깔끔하지만, 발생 가능한 예외 상황에 대한 문서화가 있으면 좋겠습니다.
다음과 같이 KDoc을 추가하는 것을 제안드립니다:
+ /** + * 리뷰를 수정합니다. + * + * @param reviewId 수정할 리뷰의 ID + * @param body 수정할 리뷰 내용 + * @return 수정 결과를 담은 Flow + * @throws IllegalArgumentException 리뷰 ID가 유효하지 않은 경우 + * @throws SecurityException 리뷰 수정 권한이 없는 경우 + */ suspend operator fun invoke( reviewId: Long, body: ModifyReviewRequest, ): Flow<BaseResponse<Void>> = reviewRepository.modifyReview(reviewId, body)app/src/main/java/com/eatssu/android/data/usecase/review/GetImageUrlUseCase.kt (1)
10-12
: 의존성 주입이 잘 구현되어 있습니다.Hilt를 사용한 생성자 주입이 깔끔하게 구현되어 있습니다. 단일 책임 원칙을 잘 따르고 있어 좋습니다.
추후 테스트 용이성을 위해 ReviewRepository를 모킹할 수 있는 테스트 모듈도 준비하시면 좋을 것 같습니다.
app/src/main/java/com/eatssu/android/data/usecase/review/GetMenuReviewListUseCase.kt (1)
10-17
: KDoc 문서화가 필요합니다.클래스와 메서드에 대한 문서화가 없습니다. 다음 내용을 포함하는 KDoc을 추가해주세요:
- 클래스의 목적
invoke
메서드의 파라미터 설명- 반환값의 의미
- 발생 가능한 예외 상황
다음과 같은 형태로 문서화를 추가해보세요:
+/** + * 메뉴에 대한 리뷰 목록을 조회하는 UseCase + */ class GetMenuReviewListUseCase @Inject constructor( private val reviewRepository: ReviewRepository, ) { + /** + * 특정 메뉴의 리뷰 목록을 조회합니다. + * + * @param menuId 조회할 메뉴의 ID + * @return 리뷰 목록이 포함된 Flow + */ suspend operator fun invoke( menuId: Long?, ): Flow<BaseResponse<GetReviewListResponse>> =app/src/main/java/com/eatssu/android/data/usecase/review/GetMealReviewListUseCase.kt (1)
13-17
: 코드 개선이 필요한 부분이 있습니다.다음 사항들을 개선하면 좋을 것 같습니다:
- 하드코딩된 값
0
의 의미가 불분명합니다. 상수로 분리하거나 설명이 필요합니다.MenuType.VARIABLE.toString()
은 불필요한 변환일 수 있습니다.다음과 같이 개선해보세요:
- suspend operator fun invoke( - mealId: Long?, - ): Flow<BaseResponse<GetReviewListResponse>> = - reviewRepository.getReviewList(MenuType.VARIABLE.toString(), mealId, 0) + private companion object { + const val DEFAULT_MENU_ID = 0L + } + + suspend operator fun invoke( + mealId: Long?, + ): Flow<BaseResponse<GetReviewListResponse>> = + reviewRepository.getReviewList( + menuType = MenuType.VARIABLE, + mealId = mealId, + menuId = DEFAULT_MENU_ID + )app/src/main/java/com/eatssu/android/data/dto/response/MenuOfMealResponse.kt (1)
21-25
: 변환 로직이 명확하게 구현되었습니다.MenusInformation에서 MenuMini로의 매핑이 잘 구현되어 있습니다. 다만, 함수의 목적을 명확히 하기 위해 KDoc 문서를 추가하면 좋을 것 같습니다.
다음과 같이 문서를 추가하는 것을 제안드립니다:
+/** + * MenusInformation을 MenuMini 모델로 변환합니다. + * @return MenuMini 객체 + */ fun MenusInformation.toMenuMini(): MenuMini { return MenuMini( id = this.menuId, name = this.name ) }app/src/main/java/com/eatssu/android/data/repository/MealRepositoryImpl.kt (2)
13-19
: 주석 처리된 코드 정리가 필요합니다.주석 처리된
getTodayMeal
메서드가 향후 구현 예정이라면 TODO 주석으로 변경하거나, 불필요한 경우 제거하는 것이 좋겠습니다.다음과 같이 변경하는 것을 제안합니다:
-// override suspend fun getTodayMeal( -// date: String, -// restaurant: String, -// time: String -// ): Flow<BaseResponse<ArrayList<GetMealResponse>>> = -// flow { emit(mealService.getTodayMeal(date, restaurant, time)) } + // TODO: Implement getTodayMeal method in future iteration + // Parameters: date, restaurant, time
20-24
: 예외 처리와 로깅 추가를 고려해주세요.현재 구현은 기능적으로는 문제가 없으나, 다음 사항들을 고려해보시면 좋겠습니다:
- 네트워크 오류나 서버 에러에 대한 예외 처리
- 디버깅을 위한 로깅 추가
다음과 같은 구현을 제안드립니다:
override suspend fun getMenuInfoByMealId(mealId: Long): Flow<BaseResponse<MenuOfMealResponse>> = flow { + try { + Timber.d("Fetching menu info for meal ID: $mealId") emit(mealService.getMenuInfoByMealId(mealId)) + } catch (e: Exception) { + Timber.e(e, "Error fetching menu info for meal ID: $mealId") + throw e + } }app/src/main/java/com/eatssu/android/data/repository/ReviewRepository.kt (2)
15-46
: 예외 처리 문서화 추가 제안각 메서드에서 발생할 수 있는 예외 상황과 에러 케이스에 대한 문서화가 필요합니다. 특히:
- 잘못된 ID 입력 시 처리
- 네트워크 오류 발생 시 처리
- 이미지 업로드 실패 시 처리
예시 문서화 형식:
/** * 리뷰를 작성합니다. * @param menuId 메뉴 ID * @param body 리뷰 작성 요청 데이터 * @return 작성 결과 * @throws IllegalArgumentException 잘못된 메뉴 ID인 경우 * @throws IOException 네트워크 오류 발생 시 */
13-13
: 테스트 용이성 개선을 위한 제안리포지토리 테스트를 위해 다음 사항들을 고려해보세요:
- 테스트용 FakeReviewRepository 구현체 추가
- 각 메서드의 반환 타입에 대한 테스트 케이스 작성
- Flow 테스트를 위한 TestCoroutineScope 활용
app/src/main/java/com/eatssu/android/di/DataModule.kt (2)
17-17
: 별칭 이름을 더 명확하게 변경하는 것이 좋습니다
ReviewRepositoryImpl1
이라는 별칭은 명확하지 않으며 향후 혼란을 야기할 수 있습니다. 더 설명적인 이름을 사용하는 것이 좋습니다.-import com.eatssu.android.data.repository.ReviewRepositoryImpl as ReviewRepositoryImpl1 +import com.eatssu.android.data.repository.ReviewRepositoryImpl
Line range hint
21-21
: 모듈에 대한 문서화가 필요합니다
DataModule
클래스에 KDoc 문서를 추가하여 모듈의 목적과 각 바인딩의 역할을 설명하는 것이 좋습니다.+/** + * 애플리케이션의 데이터 계층 의존성을 제공하는 Hilt 모듈입니다. + * 각 Repository 인터페이스와 구현체를 바인딩합니다. + */ @Module @InstallIn(SingletonComponent::class) abstract class DataModule {app/src/main/java/com/eatssu/android/ui/review/report/ReportViewModel.kt (2)
Line range hint
31-46
: 에러 처리 로직 개선이 필요합니다.현재 구현에서 몇 가지 문제점이 발견되었습니다:
onCompletion
에서 무조건error = true
로 설정되어 있어 성공 시에도 에러 상태가 됩니다.- 에러 메시지가 구체적이지 않아 사용자가 실패 원인을 파악하기 어렵습니다.
다음과 같이 수정하는 것을 제안드립니다:
viewModelScope.launch { postReportUseCase(ReportRequest(reviewId, reportType, content)).onStart { _uiState.update { it.copy(loading = true) } - }.onCompletion { - _uiState.update { it.copy(loading = false, error = true) } + }.onCompletion { error -> + _uiState.update { it.copy(loading = false) } }.catch { e -> - _uiState.update { it.copy(error = true, toastMessage = "신고가 실패하였습니다.") } + _uiState.update { + it.copy( + error = true, + toastMessage = when (e) { + is IllegalArgumentException -> "잘못된 신고 내용입니다." + is IllegalStateException -> "이미 신고한 리뷰입니다." + else -> "신고 처리 중 오류가 발생했습니다." + } + ) + } Log.e(TAG, e.toString()) }.collectLatest { result -> _uiState.update { it.copy(isDone = true, toastMessage = "신고가 완료되었습니다.") } } }
Line range hint
48-55
: UiState 초기값 검토가 필요합니다.
loading
의 초기값이true
로 설정되어 있어, ViewModel 생성 시점부터 로딩 상태로 시작됩니다. 이는 사용자 경험에 좋지 않을 수 있습니다.data class ReportUiState( - var loading: Boolean = true, + var loading: Boolean = false, var error: Boolean = false, var toastMessage: String = "", var isDone: Boolean = false, )app/src/main/java/com/eatssu/android/ui/review/delete/DeleteViewModel.kt (1)
60-68
: DeleteState 클래스의 불변성과 초기 상태 개선이 필요합니다.
- 상태 객체의 속성들이 var로 선언되어 있어 불필요하게 변경 가능합니다.
- loading의 기본값이 true로 설정되어 있어, 초기 상태가 부적절할 수 있습니다.
다음과 같이 개선해보세요:
data class DeleteState( - var loading: Boolean = true, - var error: Boolean = false, - var toastMessage: String = "", - var isDeleted: Boolean = false, + val loading: Boolean = false, + val error: Boolean = false, + val toastMessage: String = "", + val isDeleted: Boolean = false, )app/src/main/java/com/eatssu/android/data/repository/ReviewRepositoryImpl.kt (1)
19-64
:flow
대신flowOf
사용을 권장드립니다.현재 구현은 각 메서드에서
flow
빌더를 사용하여 단일 값을 방출하고 있습니다. 이 경우flowOf
를 사용하면 더 간결하고 효율적입니다.예시 코드:
- override suspend fun writeReview( + override fun writeReview( menuId: Long, body: WriteReviewRequest, ): Flow<BaseResponse<Void>> = - flow { - emit(reviewService.writeReview(menuId, body)) - } + flowOf(reviewService.writeReview(menuId, body))app/src/main/java/com/eatssu/android/ui/review/modify/ModifyViewModel.kt (1)
Line range hint
63-70
: 상태 클래스의 불변성과 초기값 개선이 필요합니다ModifyState 클래스에 다음과 같은 개선이 필요합니다:
- 프로퍼티들이
var
로 선언되어 있어 불변성이 보장되지 않습니다.loading
의 초기값이true
로 설정되어 있어, ViewModel 생성 시점부터 로딩 상태가 됩니다.다음과 같이 개선하는 것을 제안합니다:
data class ModifyState( - var loading: Boolean = true, - var error: Boolean = false, - var toastMessage: String = "", - var isDone: Boolean = false, + val loading: Boolean = false, + val error: Boolean = false, + val toastMessage: String = "", + val isDone: Boolean = false, )app/src/main/java/com/eatssu/android/ui/review/write/menu/VariableMenuViewModel.kt (2)
Line range hint
65-69
: MenuState 데이터 클래스의 가변성 제거를 고려해보세요.현재
MenuState
클래스의 프로퍼티들이var
로 선언되어 있습니다. 불변성을 보장하기 위해val
로 변경하는 것이 좋습니다.data class MenuState( - var loading: Boolean = true, - var error: Boolean = false, - var menuOfMeal: List<MenuMini>? = null, + val loading: Boolean = true, + val error: Boolean = false, + val menuOfMeal: List<MenuMini>? = null, )
32-32
: Timber 로깅 레벨을 상황에 맞게 조정하세요.현재 모든 로그가
debug
레벨로 기록되고 있습니다. 로그의 중요도에 따라 적절한 레벨을 사용하는 것이 좋습니다:
- 에러는
Timber.e()
- 경고는
Timber.w()
- 정보성 로그는
Timber.i()
-Timber.d("findMenuItemByMealId: $mealId") +Timber.i("findMenuItemByMealId: $mealId") -Timber.d("findMenuItemByMealId: onStart") +Timber.i("findMenuItemByMealId: onStart") -Timber.d("findMenuItemByMealId: onCompletion") +Timber.i("findMenuItemByMealId: onCompletion") -Timber.d("findMenuItemByMealId: catch $e") +Timber.e(e, "findMenuItemByMealId failed") -Timber.d("findMenuItemByMealId: ${result.toString()}") +Timber.i("findMenuItemByMealId: $result")Also applies to: 37-37, 41-41, 45-45, 54-54
app/src/main/java/com/eatssu/android/ui/review/write/ReviewWriteViewModel.kt (2)
Line range hint
25-50
: 초기 로딩 상태 값 검토 필요
UploadReviewState
의 초기loading
값이true
로 설정되어 있습니다. 이는 뷰모델이 생성되자마자 로딩 상태를 표시한다는 의미입니다. 실제 데이터 로딩이 시작되기 전까지는false
로 설정하는 것이 더 적절할 것 같습니다.data class UploadReviewState( var toastMessage: String = "", - var loading: Boolean = true, + var loading: Boolean = false, var error: Boolean = false, var isUpload: Boolean = false, )
Line range hint
82-86
: 상태 클래스의 불변성 개선 필요
UploadReviewState
클래스의 프로퍼티들이var
로 선언되어 있어 외부에서 직접 수정이 가능합니다. 상태 관리의 안정성을 위해val
로 변경하는 것이 좋습니다.data class UploadReviewState( - var toastMessage: String = "", - var loading: Boolean = false, - var error: Boolean = false, - var isUpload: Boolean = false, + val toastMessage: String = "", + val loading: Boolean = false, + val error: Boolean = false, + val isUpload: Boolean = false, )app/src/main/java/com/eatssu/android/ui/review/list/ReviewActivity.kt (1)
77-81
: 빈 상태 처리 로직 개선 제안개별 View의 가시성을 따로 설정하는 대신, ConstraintLayout이나 ViewGroup을 사용하여 상태별 레이아웃 전환을 더 효율적으로 관리할 수 있습니다.
- binding.llNonReview.visibility = View.VISIBLE - binding.rvReview.visibility = View.INVISIBLE + binding.viewSwitcher.displayedChild = if (it.isEmpty) 0 else 1app/src/main/res/values/strings.xml (1)
80-84
: 문자열 형식의 일관성 개선이 필요합니다.다음과 같은 비일관성이 발견되었습니다:
- "삭제 되었습니다" vs "실패하였습니다" (띄어쓰기 불일치)
- 일부 문장에서 마침표가 누락되었습니다
다음과 같이 수정을 제안드립니다:
- <string name="delete_done">리뷰가 삭제 되었습니다.</string> - <string name="delete_not">리뷰 삭제가 실패 하였습니다.</string> + <string name="delete_done">리뷰가 삭제되었습니다.</string> + <string name="delete_not">리뷰 삭제가 실패하였습니다.</string>app/src/main/java/com/eatssu/android/ui/main/menu/MenuViewModel.kt (3)
Line range hint
52-93
: 네트워크 호출 패턴 개선 필요현재 구현된
loadTodayMeal
과loadFixedMenu
함수들이 코루틴 스코프 내에서 Retrofit 콜백을 사용하고 있습니다. 이는 코루틴의 장점을 충분히 활용하지 못하는 방식입니다.다음과 같이 개선하는 것을 추천드립니다:
- fun loadTodayMeal(menuDate: String, restaurantType: Restaurant, time: Time) { - viewModelScope.launch { - mealService.getTodayMeal(menuDate, restaurantType.toString(), time.toString()) - .enqueue(object : Callback<BaseResponse<ArrayList<GetMealResponse>>> { - // ... callback implementation - }) - } - } + fun loadTodayMeal(menuDate: String, restaurantType: Restaurant, time: Time) { + viewModelScope.launch { + try { + val response = mealService.getTodayMeal(menuDate, restaurantType.toString(), time.toString()) + if (response.isSuccessful) { + val data = response.body()?.result + when (restaurantType) { + Restaurant.HAKSIK -> _todayMealDataHaksik.value = data!! + Restaurant.DODAM -> _todayMealDataDodam.value = data!! + Restaurant.DORMITORY -> _todayMealDataDormitory.value = data!! + else -> Timber.d("잘못된 식당입니다.") + } + } + } catch (e: Exception) { + Timber.e(e, "메뉴 로딩 실패") + } + } + }이렇게 수정하면:
- 코드가 더 간결해지고 가독성이 향상됩니다
- 에러 처리가 더 명확해집니다
- 코루틴의 장점을 제대로 활용할 수 있습니다
Also applies to: 94-133
Line range hint
52-93
: 로깅 구현 개선 필요현재
Log.d
를 직접 사용하고 있으며, 로그 메시지가 일관성이 없습니다.Timber 로깅 라이브러리를 사용하여 다음과 같이 개선하는 것을 추천드립니다:
- Log.d("post", "onResponse 성공" + response.body()) + Timber.d("API 호출 성공: %s", response.body()) - Log.d("post", "onFailure 에러: ${t.message}") + Timber.e(t, "API 호출 실패")장점:
- 로그 태그 자동 생성
- 예외 스택트레이스 자동 포함
- 릴리스 빌드에서 자동으로 로그 제거
Also applies to: 94-133
Line range hint
52-93
: 에러 처리 개선 필요현재 구현에서는 네트워크 오류나 서버 오류에 대한 적절한 처리가 없습니다. 사용자에게 의미 있는 피드백을 제공하지 않고 있습니다.
다음과 같은 개선을 제안드립니다:
sealed class MenuResult<out T> { data class Success<T>(val data: T) : MenuResult<T>() data class Error(val message: String) : MenuResult<Nothing>() object Loading : MenuResult<Nothing>() } fun loadTodayMeal(menuDate: String, restaurantType: Restaurant, time: Time) { viewModelScope.launch { _uiState.update { it.copy(loading = true) } try { val response = mealService.getTodayMeal(menuDate, restaurantType.toString(), time.toString()) if (response.isSuccessful) { val data = response.body()?.result when (restaurantType) { Restaurant.HAKSIK -> _todayMealDataHaksik.value = data!! Restaurant.DODAM -> _todayMealDataDodam.value = data!! Restaurant.DORMITORY -> _todayMealDataDormitory.value = data!! } _uiState.update { it.copy( loading = false, error = false, toastMessage = "" ) } } else { _uiState.update { it.copy( loading = false, error = true, toastMessage = "메뉴를 불러오는데 실패했습니다" ) } } } catch (e: Exception) { Timber.e(e, "메뉴 로딩 실패") _uiState.update { it.copy( loading = false, error = true, toastMessage = "네트워크 오류가 발생했습니다" ) } } } }이렇게 수정하면:
- 로딩 상태가 UI에 반영됩니다
- 오류 메시지가 사용자에게 표시됩니다
- 다양한 오류 상황에 대한 처리가 가능합니다
Also applies to: 94-133
app/src/main/java/com/eatssu/android/ui/review/write/ReviewWriteRateActivity.kt (6)
24-24
: Hilt 통합 및 ViewModel 주입이 올바르게 구현되었습니다.의존성 주입이 잘 구현되었으며,
by viewModels()
위임을 사용한 ViewModel 초기화가 적절합니다.추가 개선사항:
- ViewModel의 생성자에서 필요한 의존성을
@Inject
또는@HiltViewModel
로 주입하는 것을 권장합니다.- SavedStateHandle을 활용하여 프로세스 재생성 시 상태를 보존하는 것을 고려해보세요.
Also applies to: 34-34, 38-39
Line range hint
73-131
: 이미지 업로드 로직의 개선이 필요합니다.현재 구현에서 다음과 같은 문제점들이 있습니다:
- 이미지 압축 실패 시 예외 처리가 미흡합니다.
- 이미지 업로드 상태 관리가 불완전합니다.
다음과 같은 수정을 제안합니다:
private fun compressImage() { lifecycleScope.launch { try { imageFile?.let { imageFile -> compressedImage = Compressor.compress(this@ReviewWriteRateActivity, imageFile) setCompressedImage() } ?: showError("이미지를 선택해주세요") + } catch (e: Exception) { + Timber.e(e, "이미지 압축 실패") + showError("이미지 처리 중 오류가 발생했습니다") + } } } private fun setCompressedImage() { lifecycleScope.launch { try { compressedImage?.let { it -> imageviewModel.setImageFile(it) imageviewModel.saveS3() imageviewModel.uiState.collectLatest { state -> + when { + state.isError -> showError("업로드 실패: ${state.errorMessage}") + state.isImageUploadDone -> postReview() + } } } + } catch (e: Exception) { + Timber.e(e, "S3 업로드 실패") + showError("업로드 중 오류가 발생했습니다") + } } }
Line range hint
194-263
: 권한 처리 로직의 현대화가 필요합니다.Android 13(API 33) 이상에서는
READ_MEDIA_IMAGES
권한을 사용해야 합니다.다음과 같이 수정을 제안합니다:
private fun checkPermission() { when { Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU -> { when (ContextCompat.checkSelfPermission( this, Manifest.permission.READ_MEDIA_IMAGES )) { PackageManager.PERMISSION_GRANTED -> openGallery() else -> { ActivityCompat.requestPermissions( this, arrayOf(Manifest.permission.READ_MEDIA_IMAGES), PERMISSION_REQUEST_CODE ) Timber.d("권한 요청됨: READ_MEDIA_IMAGES") } } } else -> { // Android 13 미만 기기 처리 val permissions = arrayOf( Manifest.permission.READ_EXTERNAL_STORAGE, Manifest.permission.WRITE_EXTERNAL_STORAGE ) if (permissions.all { ContextCompat.checkSelfPermission(this, it) == PackageManager.PERMISSION_GRANTED }) { openGallery() } else { ActivityCompat.requestPermissions( this, permissions, PERMISSION_REQUEST_CODE ) Timber.d("권한 요청됨: 저장소") } } } }
Line range hint
314-338
: 리뷰 게시 로직의 개선이 필요합니다.상태 관리와 에러 처리를 더 견고하게 개선할 수 있습니다.
다음과 같은 수정을 제안합니다:
private fun postReview() { + if (!validateReviewData()) return viewModel.setReviewData( itemId, binding.rbMain.rating.toInt(), binding.rbAmount.rating.toInt(), binding.rbTaste.rating.toInt(), comment.toString(), imageviewModel.imageUrl.value ?: "" ) viewModel.postReview() lifecycleScope.launch { try { viewModel.uiState.collectLatest { state -> when { state.error -> handleError(state.toastMessage) !state.loading && state.isUpload -> { showToast(state.toastMessage) finish() } } } + } catch (e: Exception) { + Timber.e(e, "리뷰 게시 실패") + showToast("리뷰 게시 중 오류가 발생했습니다") + } } } +private fun validateReviewData(): Boolean { + return when { + binding.rbMain.rating == 0f || + binding.rbAmount.rating == 0f || + binding.rbTaste.rating == 0f -> { + showToast("모든 항목의 별점을 등록해주세요") + false + } + comment.isNullOrBlank() || comment!!.trim().length < REVIEW_MIN_LENGTH -> { + showToast("${REVIEW_MIN_LENGTH}자 이상 입력해주세요") + false + } + else -> true + } +}
Line range hint
280-291
: 뒤로가기 처리를 현대화해야 합니다.
onBackPressed()
가 deprecated 되었으므로OnBackPressedCallback
을 사용하도록 변경해야 합니다.다음과 같이 수정을 제안합니다:
+override fun onCreate(savedInstanceState: Bundle?) { + super.onCreate(savedInstanceState) + setupBackHandler() + // ... 기존 코드 ... +} + +private fun setupBackHandler() { + onBackPressedDispatcher.addCallback(this) { + if (imageFile?.exists() == true) { + showToast("리뷰 작성을 중지합니다") + clearImageResources() + } + finish() + } +} + +private fun clearImageResources() { + binding.ivImage.setImageDrawable(null) + imageFile?.delete() + compressedImage?.delete() +} -@Deprecated("...") -override fun onBackPressed() { - super.onBackPressed() - if (imageFile?.exists() == true) { - Toast.makeText(this, "리뷰 작성을 중지합니다.", Toast.LENGTH_SHORT).show() - binding.ivImage.setImageDrawable(null) - imageFile!!.delete() - compressedImage?.delete() - } -}
Line range hint
1-346
: 전반적인 아키텍처 개선이 필요합니다.현재 Activity가 너무 많은 책임을 가지고 있습니다. 다음과 같은 개선을 제안합니다:
- 이미지 처리 로직을 별도의 UseCase로 분리
- 권한 처리를 별도의 PermissionHandler 클래스로 분리
- UI 상태를 sealed class로 정의하여 상태 관리 개선
- 각 기능별 Contract 인터페이스 정의하여 테스트 용이성 향상
예시 구조:
sealed class ReviewWriteState { object Initial : ReviewWriteState() object Loading : ReviewWriteState() data class Error(val message: String) : ReviewWriteState() data class Success(val message: String) : ReviewWriteState() } interface ImageHandler { suspend fun compressImage(file: File): File suspend fun uploadImage(file: File): String } interface PermissionHandler { fun checkAndRequestPermissions() fun handlePermissionResult(requestCode: Int, permissions: Array<String>, grantResults: IntArray) }app/src/main/java/com/eatssu/android/data/service/ReviewService.kt (2)
42-42
: TODO 주석에 따른 페이징 라이브러리 적용 제안
//Todo paging 라이브러리 써보기
라는 주석이 있습니다. 페이징 라이브러리를 도입하여 리뷰 리스트 조회 시 효율적인 데이터 로딩 및 무한 스크롤 등의 기능을 구현하는 것을 고려해보세요。페이징 라이브러리 적용에 도움이 필요하시면 알려주세요。구현 방법이나 예제 코드를 제공해 드릴 수 있습니다。
47-47
: 주석 처리된 코드의 정리 또는 복원 권장
@Query("lastReviewId") lastReviewId: Long?,
라인이 주석 처리되어 있습니다。필요하지 않다면 코드를 제거하여 가독성을 높이고,필요하다면 주석을 해제하여 사용하시기 바랍니다。app/src/main/java/com/eatssu/android/ui/review/write/ImageViewModel.kt (2)
71-74
: 성공 시 toastMessage를 초기화해주세요이미지 업로드가 성공하면 이전에 설정된 오류 메시지가 남아 있을 수 있습니다. 성공 시
toastMessage
를 빈 문자열로 재설정하여 이러한 문제를 방지해주세요.적용 가능한 변경 사항:
_uiState.update { it.copy( loading = false, error = false, isImageUploadDone = true, imgUrl = url.toString(), + toastMessage = "" ) }
87-87
: ImageState의 loading 기본 값을 false로 설정해주세요
ImageState
의loading
속성이 기본값으로true
로 설정되어 있습니다. 일반적으로 초기 로딩 상태는false
로 설정하는 것이 적절합니다.적용 가능한 변경 사항:
-var loading: Boolean = true, +var loading: Boolean = false,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (74)
app/build.gradle.kts
(1 hunks)app/src/main/java/com/eatssu/android/App.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/dto/response/MenuOfMealResponse.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/repository/ImageRepository.kt
(0 hunks)app/src/main/java/com/eatssu/android/data/repository/ImageRepositoryImpl.kt
(0 hunks)app/src/main/java/com/eatssu/android/data/repository/MealRepository.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/repository/MealRepositoryImpl.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/repository/ReviewRepository.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/repository/ReviewRepositoryImpl.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/service/ImageService.kt
(0 hunks)app/src/main/java/com/eatssu/android/data/service/MealService.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/service/ReviewService.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/usecase/alarm/AlarmUsecase.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/usecase/alarm/GetDailyNotificationStatusUseCase.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/usecase/alarm/SetDailyNotificationStatusUseCase.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/usecase/auth/GetAccessTokenUseCase.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/usecase/auth/GetMyReviewsUseCase.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/usecase/auth/GetRefreshTokenUseCase.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/usecase/auth/GetUserEmailUseCase.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/usecase/auth/GetUserInfoUseCase.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/usecase/auth/GetUserNameUseCase.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/usecase/auth/LoginUseCase.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/usecase/auth/LogoutUseCase.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/usecase/auth/ReissueTokenUseCase.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/usecase/auth/SetAccessTokenUseCase.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/usecase/auth/SetRefreshTokenUseCase.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/usecase/auth/SetUserEmailUseCase.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/usecase/auth/SetUserNameUseCase.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/usecase/auth/SignOutUseCase.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/usecase/auth/ValidateUserNameUseCase.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/usecase/menu/GetMenuNameListOfMealUseCase.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/usecase/review/DeleteReviewUseCase.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/usecase/review/GetImageUrlUseCase.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/usecase/review/GetMealReviewInfoUseCase.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/usecase/review/GetMealReviewListUseCase.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/usecase/review/GetMenuReviewInfoUseCase.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/usecase/review/GetMenuReviewListUseCase.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/usecase/review/ModifyReviewUseCase.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/usecase/review/PostReportUseCase.kt
(1 hunks)app/src/main/java/com/eatssu/android/data/usecase/review/WriteReviewUseCase.kt
(1 hunks)app/src/main/java/com/eatssu/android/di/DataModule.kt
(2 hunks)app/src/main/java/com/eatssu/android/di/NetworkModule.kt
(2 hunks)app/src/main/java/com/eatssu/android/di/network/TokenInterceptor.kt
(5 hunks)app/src/main/java/com/eatssu/android/ui/login/IntroViewModel.kt
(1 hunks)app/src/main/java/com/eatssu/android/ui/login/LoginViewModel.kt
(1 hunks)app/src/main/java/com/eatssu/android/ui/main/MainViewModel.kt
(1 hunks)app/src/main/java/com/eatssu/android/ui/main/menu/MenuViewModel.kt
(1 hunks)app/src/main/java/com/eatssu/android/ui/mypage/MyPageViewModel.kt
(4 hunks)app/src/main/java/com/eatssu/android/ui/mypage/SignOutViewModel.kt
(1 hunks)app/src/main/java/com/eatssu/android/ui/mypage/myreview/MyReviewAdapter.kt
(3 hunks)app/src/main/java/com/eatssu/android/ui/mypage/myreview/MyReviewViewModel.kt
(3 hunks)app/src/main/java/com/eatssu/android/ui/mypage/terms/WebViewActivity.kt
(2 hunks)app/src/main/java/com/eatssu/android/ui/mypage/usernamechange/UserNameChangeViewModel.kt
(3 hunks)app/src/main/java/com/eatssu/android/ui/review/delete/DeleteViewModel.kt
(1 hunks)app/src/main/java/com/eatssu/android/ui/review/delete/MyReviewDialogActivity.kt
(3 hunks)app/src/main/java/com/eatssu/android/ui/review/list/ReviewActivity.kt
(5 hunks)app/src/main/java/com/eatssu/android/ui/review/list/ReviewViewModel.kt
(1 hunks)app/src/main/java/com/eatssu/android/ui/review/list/ReviewViewModelFactory.kt
(0 hunks)app/src/main/java/com/eatssu/android/ui/review/modify/ModifyReviewActivity.kt
(4 hunks)app/src/main/java/com/eatssu/android/ui/review/modify/ModifyViewModel.kt
(1 hunks)app/src/main/java/com/eatssu/android/ui/review/report/OthersReviewDialogActivity.kt
(2 hunks)app/src/main/java/com/eatssu/android/ui/review/report/ReportViewModel.kt
(1 hunks)app/src/main/java/com/eatssu/android/ui/review/write/ImageViewModel.kt
(3 hunks)app/src/main/java/com/eatssu/android/ui/review/write/ImageViewModelFactory.kt
(0 hunks)app/src/main/java/com/eatssu/android/ui/review/write/ReviewWriteRateActivity.kt
(12 hunks)app/src/main/java/com/eatssu/android/ui/review/write/ReviewWriteViewModel.kt
(2 hunks)app/src/main/java/com/eatssu/android/ui/review/write/ReviewWriteViewModelFactory.kt
(0 hunks)app/src/main/java/com/eatssu/android/ui/review/write/menu/ReviewWriteMenuActivity.kt
(2 hunks)app/src/main/java/com/eatssu/android/ui/review/write/menu/VariableMenuModelFactory.kt
(0 hunks)app/src/main/java/com/eatssu/android/ui/review/write/menu/VariableMenuPickAdapter.kt
(3 hunks)app/src/main/java/com/eatssu/android/ui/review/write/menu/VariableMenuViewModel.kt
(1 hunks)app/src/main/res/layout/activity_my_review_dialog.xml
(1 hunks)app/src/main/res/menu/menu_my_review.xml
(1 hunks)app/src/main/res/values/strings.xml
(1 hunks)
💤 Files with no reviewable changes (7)
- app/src/main/java/com/eatssu/android/data/repository/ImageRepository.kt
- app/src/main/java/com/eatssu/android/data/repository/ImageRepositoryImpl.kt
- app/src/main/java/com/eatssu/android/data/service/ImageService.kt
- app/src/main/java/com/eatssu/android/ui/review/list/ReviewViewModelFactory.kt
- app/src/main/java/com/eatssu/android/ui/review/write/ImageViewModelFactory.kt
- app/src/main/java/com/eatssu/android/ui/review/write/ReviewWriteViewModelFactory.kt
- app/src/main/java/com/eatssu/android/ui/review/write/menu/VariableMenuModelFactory.kt
✅ Files skipped from review due to trivial changes (18)
- app/src/main/java/com/eatssu/android/data/usecase/alarm/AlarmUsecase.kt
- app/src/main/java/com/eatssu/android/data/usecase/alarm/GetDailyNotificationStatusUseCase.kt
- app/src/main/java/com/eatssu/android/data/usecase/alarm/SetDailyNotificationStatusUseCase.kt
- app/src/main/java/com/eatssu/android/data/usecase/auth/GetMyReviewsUseCase.kt
- app/src/main/java/com/eatssu/android/data/usecase/auth/GetUserEmailUseCase.kt
- app/src/main/java/com/eatssu/android/data/usecase/auth/GetUserInfoUseCase.kt
- app/src/main/java/com/eatssu/android/data/usecase/auth/LoginUseCase.kt
- app/src/main/java/com/eatssu/android/data/usecase/auth/LogoutUseCase.kt
- app/src/main/java/com/eatssu/android/data/usecase/auth/ReissueTokenUseCase.kt
- app/src/main/java/com/eatssu/android/data/usecase/auth/SetUserEmailUseCase.kt
- app/src/main/java/com/eatssu/android/data/usecase/auth/SetUserNameUseCase.kt
- app/src/main/java/com/eatssu/android/data/usecase/auth/SignOutUseCase.kt
- app/src/main/java/com/eatssu/android/data/usecase/auth/ValidateUserNameUseCase.kt
- app/src/main/java/com/eatssu/android/data/usecase/review/PostReportUseCase.kt
- app/src/main/java/com/eatssu/android/ui/login/IntroViewModel.kt
- app/src/main/java/com/eatssu/android/ui/login/LoginViewModel.kt
- app/src/main/java/com/eatssu/android/ui/main/MainViewModel.kt
- app/src/main/java/com/eatssu/android/ui/mypage/SignOutViewModel.kt
🚧 Files skipped from review as they are similar to previous changes (12)
- app/src/main/java/com/eatssu/android/App.kt
- app/src/main/java/com/eatssu/android/di/network/TokenInterceptor.kt
- app/src/main/java/com/eatssu/android/ui/mypage/MyPageViewModel.kt
- app/src/main/java/com/eatssu/android/ui/mypage/myreview/MyReviewAdapter.kt
- app/src/main/java/com/eatssu/android/ui/mypage/myreview/MyReviewViewModel.kt
- app/src/main/java/com/eatssu/android/ui/mypage/terms/WebViewActivity.kt
- app/src/main/java/com/eatssu/android/ui/mypage/usernamechange/UserNameChangeViewModel.kt
- app/src/main/java/com/eatssu/android/ui/review/modify/ModifyReviewActivity.kt
- app/src/main/java/com/eatssu/android/ui/review/report/OthersReviewDialogActivity.kt
- app/src/main/java/com/eatssu/android/ui/review/write/menu/ReviewWriteMenuActivity.kt
- app/src/main/res/layout/activity_my_review_dialog.xml
- app/src/main/res/menu/menu_my_review.xml
🔇 Additional comments (56)
app/src/main/java/com/eatssu/android/data/usecase/review/DeleteReviewUseCase.kt (1)
8-13
: 클린 아키텍처 원칙을 잘 준수하였습니다!
UseCase가 단일 책임 원칙을 잘 따르고 있으며, 의존성 주입을 적절히 활용하고 있습니다.
app/src/main/java/com/eatssu/android/data/usecase/auth/GetUserNameUseCase.kt (1)
Line range hint 1-14
: 의존성 검증 필요
Hilt를 사용한 의존성 주입이 올바르게 설정되어 있는지 확인이 필요합니다.
✅ Verification successful
Let me check the AppModule configuration to verify the Hilt dependency injection setup.
Let me check one more thing about the PreferencesRepository and MySharedPreferences usage.
Hilt 의존성 주입 설정이 올바르게 되어 있습니다
GetUserNameUseCase의 의존성 주입이 다음과 같이 정상적으로 구성되어 있음을 확인했습니다:
- AppModule에서 Context가 올바르게 제공되고 있습니다
- UserNameChangeViewModel에서 GetUserNameUseCase가 정상적으로 주입되어 사용되고 있습니다
- MySharedPreferences를 통한 데이터 저장 구현이 적절합니다
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Hilt 모듈 설정 확인
echo "Checking Hilt module configuration..."
rg -l "PreferencesRepository|GetUserNameUseCase" --type kotlin
# ApplicationComponent 설정 확인
ast-grep --pattern 'class $_ {
$$$
@Provides
$$$
fun provideContext($$$): Context {
$$$
}
$$$
}'
Length of output: 1055
Script:
#!/bin/bash
# AppModule의 구체적인 내용 확인
echo "Checking AppModule content..."
cat app/src/main/java/com/eatssu/android/di/AppModule.kt
# GetUserNameUseCase 사용처 확인
echo -e "\nChecking GetUserNameUseCase usage..."
rg "GetUserNameUseCase" -B 2 -A 2
Length of output: 3236
Script:
#!/bin/bash
# PreferencesRepository와 MySharedPreferences 구현 확인
echo "Checking PreferencesRepository implementation..."
cat app/src/main/java/com/eatssu/android/data/repository/PreferencesRepository.kt
echo -e "\nChecking MySharedPreferences implementation..."
cat app/src/main/java/com/eatssu/android/util/MySharedPreferences.kt
Length of output: 5202
app/src/main/java/com/eatssu/android/data/usecase/auth/GetRefreshTokenUseCase.kt (2)
Line range hint 1-7
: LGTM! 패키지 구조와 의존성 주입이 잘 구성되어 있습니다.
Hilt를 사용한 의존성 주입과 인증 관련 usecase들을 auth 패키지로 분리한 것이 깔끔해 보입니다.
Line range hint 12-14
: suspend 함수의 필요성을 검토해주세요.
현재 구현에서는 동기적인 SharedPreferences 접근만 수행하고 있어 suspend 함수가 필요하지 않을 수 있습니다. 하지만 추후 Repository 패턴 적용 시 비동기 작업이 필요할 수 있으므로, suspend 키워드는 유지하는 것이 좋아 보입니다.
다음 스크립트로 다른 인증 관련 UseCase들의 비동기 처리 패턴을 확인해보시기 바랍니다:
✅ Verification successful
suspend 키워드 유지가 적절합니다.
코드베이스 분석 결과, 다음과 같은 이유로 suspend 키워드를 유지하는 것이 바람직합니다:
- 모든 인증 관련 UseCase들이 일관되게 suspend 함수로 구현되어 있습니다.
- 주석된 코드에서
PreferencesRepository
패턴으로의 마이그레이션이 계획되어 있음이 확인됩니다. GetMyReviewsUseCase
,GetUserInfoUseCase
등 실제로 비동기 작업이 필요한 UseCase들과의 일관성을 유지할 수 있습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check async patterns in other auth use cases
ast-grep --pattern 'class $_UseCase {
suspend operator fun invoke($$$) {
$$$
}
}'
Length of output: 91
Script:
#!/bin/bash
# Search for auth-related use case files
fd ".*UseCase.*\.kt" app/src/main/java/com/eatssu/android/data/usecase/auth
# Check implementation of auth-related use cases
for file in $(fd ".*UseCase.*\.kt" app/src/main/java/com/eatssu/android/data/usecase/auth); do
echo "=== $file ==="
cat "$file"
done
Length of output: 14104
app/src/main/java/com/eatssu/android/data/usecase/menu/GetMenuNameListOfMealUseCase.kt (1)
1-8
: 패키지 구조와 임포트가 적절합니다!
Clean Architecture의 원칙에 따라 패키지 구조가 잘 구성되어 있으며, 필요한 임포트들이 모두 포함되어 있습니다.
app/src/main/java/com/eatssu/android/data/usecase/review/WriteReviewUseCase.kt (2)
9-11
: 클린 아키텍처와 의존성 주입이 잘 구현되었습니다!
Hilt를 사용한 의존성 주입과 단일 책임 원칙을 잘 준수하고 있습니다. ReviewRepository
를 생성자 주입으로 받아 테스트 용이성도 확보했습니다.
1-14
: 관련 클래스들의 구현 여부를 확인해주세요.
UseCase의 의존성과 사용처를 확인할 필요가 있습니다.
✅ Verification successful
모든 관련 클래스들이 올바르게 구현되어 있습니다.
- WriteReviewRequest는 data class로 적절히 구현되어 있으며, 필요한 필드들(mainRating, amountRating, tasteRating, content)을 포함하고 있습니다.
- ReviewRepository 인터페이스가 존재하며 writeReview 메소드가 정의되어 있습니다.
- ReviewWriteViewModel에서 WriteReviewUseCase가 의존성으로 주입되어 사용되고 있습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence and implementation of related classes
# Check WriteReviewRequest implementation
echo "Checking WriteReviewRequest implementation..."
ast-grep --pattern 'class WriteReviewRequest'
# Check ReviewRepository interface
echo "Checking ReviewRepository interface..."
ast-grep --pattern 'interface ReviewRepository'
# Check ViewModel usage
echo "Checking ViewModel usage of WriteReviewUseCase..."
rg "private.*WriteReviewUseCase" -t kt
Length of output: 3965
Script:
#!/bin/bash
# Let's try different approaches to find the required information
# Check WriteReviewRequest data class
echo "Checking WriteReviewRequest data class..."
rg "data class WriteReviewRequest" --type-list
rg "data class WriteReviewRequest" -A 5
# Check ViewModel usage with different pattern
echo "Checking ViewModel usage..."
rg "WriteReviewUseCase" -A 2 --type kotlin
# Check if there are any files containing WriteReview in their name
echo "Checking files with WriteReview in name..."
fd WriteReview
Length of output: 2737
app/src/main/java/com/eatssu/android/data/usecase/review/GetMealReviewInfoUseCase.kt (2)
1-8
: 패키지 구조와 임포트가 적절합니다!
Clean Architecture 원칙에 따라 올바른 계층에 위치해 있으며, 필요한 모든 의존성이 잘 임포트되어 있습니다.
12-14
: 함수 구현이 간결하고 명확합니다!
Flow와 코루틴을 적절히 활용한 리액티브 프로그래밍 구현이 잘 되어있습니다. 다만, 에러 처리가 Repository 계층에서 적절히 구현되어 있는지 확인이 필요합니다.
Repository 구현체에서 에러 처리가 되어있는지 확인해보겠습니다:
✅ Verification successful
Let me check the error handling implementation in the BaseResponse class and reviewService.
Let me check the ReviewService interface and implementation to understand the error handling better.
Let me check one more time for the getMealReviewInfo endpoint in the service.
에러 처리가 적절히 구현되어 있습니다
BaseResponse 클래스를 통해 서버 응답의 성공/실패 여부와 에러 메시지를 처리하고 있으며, Repository 계층에서 Flow를 사용하여 비동기 스트림을 적절히 처리하고 있습니다. 추가적인 에러 처리가 필요하지 않습니다.
BaseResponse
는isSuccess
,code
,message
필드를 통해 기본적인 에러 처리를 제공ReviewRepositoryImpl
에서flow
블록을 사용하여 비동기 처리ReviewService
의 suspend 함수를 통해 코루틴 컨텍스트에서 안전하게 실행
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# ReviewRepository 구현체에서 에러 처리 확인
rg -A 10 "getMealReviewInfo.*Flow.*Response" app/src/main/java/com/eatssu/android/data/repository/
Length of output: 1332
Script:
#!/bin/bash
# Check BaseResponse implementation
rg -A 5 "class BaseResponse" app/src/main/java/com/eatssu/android/
# Check ReviewService implementation
rg -A 5 "getMealReviewInfo.*Response" app/src/main/java/com/eatssu/android/data/service/
Length of output: 766
Script:
#!/bin/bash
# Check ReviewService interface and implementation
rg -A 5 "interface ReviewService" app/src/main/java/com/eatssu/android/
rg -A 5 "@GET.*getMealReviewInfo" app/src/main/java/com/eatssu/android/
Length of output: 756
Script:
#!/bin/bash
# Check for getMealReviewInfo endpoint with different patterns
rg -A 5 "getMealReviewInfo" app/src/main/java/com/eatssu/android/data/service/ReviewService.kt
Length of output: 278
app/src/main/java/com/eatssu/android/data/usecase/review/GetMenuReviewInfoUseCase.kt (1)
9-11
: 클린 아키텍처 원칙을 잘 준수한 구현입니다! 👍
Hilt를 사용한 의존성 주입과 단일 책임 원칙을 잘 따르고 있습니다. ReviewRepository
를 생성자 주입으로 받아 테스트 용이성을 높인 점이 좋습니다.
app/src/main/java/com/eatssu/android/data/usecase/review/ModifyReviewUseCase.kt (2)
1-8
: 패키지 구조와 임포트가 깔끔합니다!
클린 아키텍처 패턴에 맞게 잘 구성되어 있습니다.
9-11
: 의존성 주입이 잘 구현되어 있습니다!
Hilt를 사용한 의존성 주입이 깔끔하게 구현되어 있습니다. 단일 책임 원칙을 잘 따르고 있습니다.
리포지토리 구현체가 올바르게 바인딩되어 있는지 확인하겠습니다:
✅ Verification successful
의존성 주입이 올바르게 구현되어 있음을 확인했습니다!
ReviewRepository 인터페이스가 정의되어 있고, ReviewRepositoryImpl이 이를 구현하며, DataModule에서 @BINDS를 통해 적절하게 바인딩되어 있습니다. Hilt의 의존성 주입 패턴을 정확하게 따르고 있습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify ReviewRepository binding in Hilt module
ast-grep --pattern 'class $_ {
$$$
@Binds
$$$
fun $_($_: ReviewRepositoryImpl): ReviewRepository
$$$
}'
Length of output: 115
Script:
#!/bin/bash
# Search for ReviewRepository interface definition
ast-grep --pattern 'interface ReviewRepository {
$$$
}'
# Search for ReviewRepositoryImpl class
ast-grep --pattern 'class ReviewRepositoryImpl $$$implements ReviewRepository {
$$$
}'
# Search for Hilt module with different binding pattern
ast-grep --pattern '@Module
$$$
class $_ {
$$$
@Provides
fun provideReviewRepository($_): ReviewRepository {
$$$
}
$$$
}'
# Search for any ReviewRepository related bindings
rg "ReviewRepository" -A 3 -B 3
Length of output: 19026
app/src/main/java/com/eatssu/android/data/usecase/review/GetImageUrlUseCase.kt (2)
1-9
: 패키지 구조와 임포트가 깔끔합니다!
클린 아키텍처 원칙에 따라 적절한 패키지 구조를 가지고 있으며, 필요한 임포트들이 잘 정리되어 있습니다.
13-17
: 에러 처리 검증이 필요합니다.
Flow를 사용한 비동기 처리가 잘 구현되어 있습니다. 다만, 이미지 업로드 실패 시의 에러 처리가 ReviewRepository에서 적절히 이루어지는지 확인이 필요합니다.
리포지토리의 에러 처리를 확인하기 위해 다음 스크립트를 실행해 주세요:
✅ Verification successful
에러 처리가 적절히 구현되어 있습니다.
ReviewRepositoryImpl에서 Flow를 사용하여 이미지 업로드 요청을 처리하고 있으며, 상위 ImageViewModel에서 catch 블록을 통해 에러를 적절히 처리하고 있습니다:
// ReviewRepositoryImpl.kt
override suspend fun getImageString(image: MultipartBody.Part): Flow<BaseResponse<ImageResponse>> =
flow {
emit(reviewService.uploadImage(image))
}
// ImageViewModel.kt
}.catch { e ->
_uiState.update { it.copy(error = true, toastMessage = "이미지 변환에 실패하였습니다.") }
Timber.e(e.toString())
}
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# ReviewRepository와 ReviewRepositoryImpl에서 에러 처리 로직 확인
# Flow 에러 처리 패턴 검색
ast-grep --pattern 'catch { $$ }'
# BaseResponse 에러 처리 패턴 검색
rg -A 5 'BaseResponse\.Error'
Length of output: 67
Script:
#!/bin/bash
# ReviewRepository 인터페이스와 구현체 찾기
fd "ReviewRepository" --type f
# 이미지 업로드 관련 에러 처리 코드 검색
rg -A 10 "getImageString|uploadImage"
# Flow 관련 에러 처리 패턴 검색 (다양한 패턴)
rg -A 5 "catch|onError|emit.*Error"
# BaseResponse 관련 코드 검색
rg -A 5 "BaseResponse"
Length of output: 93271
app/src/main/java/com/eatssu/android/data/usecase/review/GetMenuReviewListUseCase.kt (2)
10-12
: 클래스 구조가 깔끔하고 DI 패턴이 잘 적용되었습니다!
Hilt를 사용한 의존성 주입이 잘 구현되어 있습니다.
13-17
: 하드코딩된 값들에 대한 설명이 필요합니다.
MenuType.FIXED
와 mealId: 0
이 하드코딩되어 있는데, 이러한 값들을 사용하는 이유와 의미를 설명해 주시면 좋겠습니다. 만약 이 값들이 변경될 수 있다면, 파라미터로 받아오는 것이 더 좋을 것 같습니다.
다음 스크립트로 다른 곳에서도 같은 패턴이 사용되는지 확인해보겠습니다:
✅ Verification successful
Let me gather more context about the MenuType enum and related usages.
하드코딩된 값들의 의미가 명확해졌습니다.
MenuType.FIXED
와 mealId: 0
은 고정 메뉴의 리뷰를 조회하기 위한 의도적인 값입니다:
MenuType.FIXED
는 푸드코트, 스낵코너와 같은 고정 메뉴를 제공하는 식당을 구분하는 값입니다.mealId: 0
은 고정 메뉴의 경우 식사 ID가 필요하지 않기 때문에 사용되는 기본값입니다.
이는 GetMealReviewListUseCase
에서 가변 메뉴(MenuType.VARIABLE
)의 리뷰를 조회할 때와 대조되는 패턴으로, 의도된 설계입니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# MenuType.FIXED와 mealId: 0 사용 패턴 검색
rg -A 2 "MenuType\.FIXED\.toString\(\)"
rg "getReviewList.*,\s*0\s*,"
Length of output: 785
Script:
#!/bin/bash
# Search for MenuType enum definition and usages
ast-grep --pattern 'enum class MenuType'
rg -A 5 "MenuType"
# Search for getReviewList implementation and usages
ast-grep --pattern 'fun getReviewList'
Length of output: 14198
app/src/main/java/com/eatssu/android/data/usecase/review/GetMealReviewListUseCase.kt (3)
1-9
: LGTM! 패키지 구조와 임포트가 적절합니다.
클린 아키텍처 원칙에 따라 올바르게 구성되어 있습니다.
10-12
: 의존성 주입이 올바르게 구현되었습니다.
Hilt를 사용한 생성자 주입이 잘 구현되어 있으며, 단일 책임 원칙을 잘 따르고 있습니다.
1-17
: PR 목표에 맞게 잘 구현되었습니다.
- Hilt를 사용한 의존성 주입 ✓
- Flow를 활용한 비동기 처리 ✓
- 클린 아키텍처 원칙 준수 ✓
전반적으로 리팩토링 목표를 잘 달성했습니다.
app/src/main/java/com/eatssu/android/data/dto/response/MenuOfMealResponse.kt (1)
17-19
: 코드가 깔끔하고 가독성이 좋습니다!
Kotlin의 map
함수를 활용하여 간결하고 효율적으로 구현되었습니다.
app/src/main/java/com/eatssu/android/data/service/MealService.kt (1)
20-22
: 코루틴으로의 전환이 잘 이루어졌습니다! 👍
Retrofit의 Call
래퍼를 제거하고 suspend 함수로 변경한 것은 좋은 접근입니다. 이는 코루틴을 활용한 더 나은 비동기 처리를 가능하게 합니다.
다른 서비스들도 같은 패턴으로 리팩토링되었는지 확인해보겠습니다:
✅ Verification successful
Based on the results, I can now generate the final response since we have clear evidence that:
- No methods are using
Call<>
type anymore (empty results from the first search) - All service interfaces consistently use suspend functions (as shown in the second search results)
코루틴 리팩토링이 전체 서비스에 걸쳐 일관되게 적용되었습니다 ✨
모든 서비스 인터페이스가 suspend
함수를 사용하도록 성공적으로 변환되었으며, Call<>
타입을 사용하는 메서드가 더 이상 없음을 확인했습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: 다른 서비스 인터페이스들의 패턴을 확인합니다.
# 서비스 인터페이스들에서 아직 Call을 사용하는 메서드들을 검색
echo "Call 타입을 여전히 사용 중인 서비스 메서드들:"
rg "fun\s+\w+.*:\s*Call<" "app/src/main/java/com/eatssu/android/data/service/"
# suspend 키워드를 사용하는 메서드들을 검색
echo -e "\nsuspend 키워드를 사용하는 서비스 메서드들:"
rg "suspend\s+fun" "app/src/main/java/com/eatssu/android/data/service/"
Length of output: 1926
app/src/main/java/com/eatssu/android/data/repository/MealRepositoryImpl.kt (1)
1-11
: 클래스 구조가 잘 설계되었습니다!
의존성 주입과 리포지토리 패턴이 Clean Architecture 원칙에 맞게 잘 구현되어 있습니다.
app/src/main/java/com/eatssu/android/data/repository/ReviewRepository.kt (2)
3-13
: 클린 아키텍처 원칙을 잘 따르고 있습니다!
리포지토리 인터페이스가 데이터 계층과 도메인 계층을 잘 분리하고 있습니다.
15-42
: Flow를 반환하는 메서드에서 suspend 키워드 제거 필요
이전 리뷰 코멘트가 여전히 유효합니다. Flow는 이미 비동기 스트림을 표현하므로, suspend 키워드가 불필요합니다.
app/src/main/java/com/eatssu/android/di/DataModule.kt (2)
43-46
: MealRepository 구현 검증이 필요합니다
새로운 MealRepository 바인딩이 추가되었습니다. 구현이 올바르게 되어있는지 확인이 필요합니다.
✅ Verification successful
MealRepository 구현이 올바르게 되어 있습니다
MealRepositoryImpl이 MealRepository 인터페이스를 정확하게 구현하고 있습니다:
getMenuInfoByMealId
메서드가 올바르게 구현되어 있습니다getTodayMeal
메서드는 현재 주석 처리되어 있지만, 구현 코드가 준비되어 있습니다- 의존성 주입(@Inject)이 적절하게 설정되어 있습니다
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# MealRepository 인터페이스의 모든 메서드가 구현되어 있는지 확인
ast-grep --pattern 'interface MealRepository {
$$$
}'
# MealRepositoryImpl의 구현 확인
ast-grep --pattern 'class MealRepositoryImpl : MealRepository {
$$$
}'
Length of output: 1425
Script:
#!/bin/bash
# Search for MealRepositoryImpl implementation
rg -U "class MealRepositoryImpl.*implements.*MealRepository" -A 20
# Alternative search using ripgrep
rg -U "class MealRepositoryImpl" -A 20
Length of output: 1778
38-41
: ReviewRepository 구현 검증이 필요합니다
새로운 ReviewRepository 바인딩이 추가되었습니다. 구현이 올바르게 되어있는지 확인이 필요합니다.
✅ Verification successful
ReviewRepository 구현이 정상적으로 확인되었습니다
ReviewRepositoryImpl은 ReviewRepository 인터페이스의 모든 메서드(writeReview, deleteReview)를 올바르게 구현하고 있으며, DataModule에서는 이를 ReviewRepositoryImpl1이라는 별칭으로 임포트하여 바인딩하고 있습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# ReviewRepository 인터페이스의 모든 메서드가 구현되어 있는지 확인
ast-grep --pattern 'interface ReviewRepository {
$$$
}'
# ReviewRepositoryImpl의 구현 확인
ast-grep --pattern 'class ReviewRepositoryImpl : ReviewRepository {
$$$
}'
Length of output: 1425
Script:
#!/bin/bash
# Search for ReviewRepository interface
rg -A 10 "interface ReviewRepository"
# Search for ReviewRepositoryImpl1 class
rg -A 10 "class ReviewRepositoryImpl1"
# Search for any files containing ReviewRepository in their name
fd "ReviewRepository"
Length of output: 1290
Script:
#!/bin/bash
# Check ReviewRepositoryImpl implementation
rg -A 15 "class ReviewRepositoryImpl" app/src/main/java/com/eatssu/android/data/repository/ReviewRepositoryImpl.kt
# Check if ReviewRepositoryImpl1 exists in the codebase
rg "ReviewRepositoryImpl1"
Length of output: 882
app/src/main/java/com/eatssu/android/ui/review/write/menu/VariableMenuPickAdapter.kt (4)
9-10
: 생성자의 null 처리가 적절히 구현되었습니다.
null 가능한 menuList
를 처리할 수 있도록 변경된 것이 좋습니다.
18-23
: ViewHolder의 bind 메서드가 안전하게 구현되었습니다.
null 안전 연산자를 적절히 사용하여 menuItem
과 관련 속성에 접근하고 있습니다.
37-37
: getItemCount의 null 처리가 올바르게 구현되었습니다.
Elvis 연산자를 사용하여 menuList
가 null일 경우 0을 반환하도록 처리한 것이 적절합니다.
39-41
: getItem 메서드의 반환 타입이 적절히 수정되었습니다.
null 가능성을 고려하여 Pair<String?, Long?>
로 변경된 것이 안전합니다.
app/src/main/java/com/eatssu/android/ui/review/report/ReportViewModel.kt (1)
7-7
: 패키지 구조 개선이 잘 되었습니다!
리뷰 관련 유스케이스들을 review
패키지로 이동하여 도메인 기능별로 구조화한 것이 좋습니다.
app/src/main/java/com/eatssu/android/ui/review/delete/DeleteViewModel.kt (2)
22-26
: 의존성 주입 구현이 잘 되어있습니다!
Hilt를 사용한 의존성 주입이 Clean Architecture 원칙에 맞게 잘 구현되어 있습니다.
28-29
: 상태 관리 구현이 적절합니다!
MutableStateFlow와 StateFlow를 사용한 상태 관리가 MVVM 패턴에 맞게 잘 구현되어 있습니다.
app/src/main/java/com/eatssu/android/data/repository/ReviewRepositoryImpl.kt (1)
1-18
: 클래스 구조와 의존성 주입이 잘 구현되었습니다!
Hilt를 사용한 의존성 주입이 Clean Architecture 원칙에 따라 적절하게 구현되었습니다.
app/src/main/java/com/eatssu/android/ui/review/modify/ModifyViewModel.kt (1)
22-25
: 의존성 주입 구현이 잘 되어있습니다!
Hilt를 사용한 의존성 주입이 깔끔하게 구현되어 있으며, MVVM 아키텍처 패턴을 잘 따르고 있습니다.
app/src/main/java/com/eatssu/android/ui/review/write/menu/VariableMenuViewModel.kt (1)
23-26
: Hilt 의존성 주입이 올바르게 구현되었습니다.
@HiltViewModel
과 생성자의 @Inject
어노테이션이 적절하게 사용되었습니다. Clean Architecture 원칙에 따라 UseCase를 주입받아 사용하는 것이 좋은 구현 방식입니다.
app/src/main/java/com/eatssu/android/ui/review/write/ReviewWriteViewModel.kt (1)
21-24
: Hilt 의존성 주입이 올바르게 구현되었습니다
@HiltViewModel
어노테이션과 생성자 주입이 Clean Architecture 원칙에 따라 적절하게 구현되었습니다.
app/src/main/java/com/eatssu/android/ui/review/delete/MyReviewDialogActivity.kt (3)
5-19
: 의존성 주입 설정이 올바르게 구현되었습니다.
Hilt를 통한 의존성 주입 설정과 필요한 import 문이 적절하게 추가되었습니다.
23-23
: ViewModel 초기화가 개선되었습니다.
by viewModels()
위임 속성을 사용하여 ViewModel 초기화를 더 간단하고 안전하게 구현하였습니다.
71-71
: 리뷰 삭제 로직 검증이 필요합니다.
리뷰 삭제 후 적절한 에러 처리와 사용자 피드백이 구현되어 있는지 확인이 필요합니다.
✅ Verification successful
리뷰 삭제 로직이 적절히 구현되어 있습니다.
DeleteViewModel에서 다음과 같이 에러 처리와 사용자 피드백이 구현되어 있습니다:
- 삭제 실패 시 에러 상태 업데이트 및 실패 메시지 표시
- 삭제 성공 시 성공 상태 업데이트 및 완료 메시지 표시
- 로딩 상태 관리를 통한 사용자 피드백
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 리뷰 삭제 관련 에러 처리 코드 검색
rg -A 5 "deleteReview.*catch|onError"
# 삭제 실패 시 사용자 피드백 관련 코드 검색
ast-grep --pattern 'fun deleteReview($$$) {
$$$
catch($$$) {
$$$
}
}'
Length of output: 121
Script:
#!/bin/bash
# 리뷰 삭제 관련 코드 검색
rg -A 10 "deleteReview"
# ViewModel에서 에러 처리 관련 코드 검색
rg -A 10 "class.*ReviewViewModel"
# UI 상태 관련 코드 검색
rg -A 5 "_uiState|UiState.*review"
Length of output: 65000
app/src/main/java/com/eatssu/android/di/NetworkModule.kt (2)
6-6
: 새로운 서비스 import 추가가 적절합니다.
새로 추가된 MealService와 ReviewService의 import가 기존 패턴을 잘 따르고 있습니다.
Also applies to: 9-9
87-97
: 서비스 프로바이더 구현이 잘 되었습니다.
Hilt DI 패턴을 일관성 있게 잘 따르고 있습니다. 다만, 리팩토링 목적에 맞게 다음 사항들을 확인해 주시면 좋겠습니다:
- ReviewService가 Flow를 활용하도록 구현되어 있는지
- Clean Architecture 원칙에 따라 Repository 계층과 잘 통합되어 있는지
아래 스크립트로 서비스 구현을 확인해보겠습니다:
✅ Verification successful
Let me gather more information about the repository implementation.
서비스 프로바이더와 리포지토리 구현이 모두 검증되었습니다.
검증 결과 다음 사항들이 확인되었습니다:
- ReviewService가 적절하게 구현되어 있으며 필요한 모든 API 엔드포인트가 정의되어 있습니다.
- ReviewRepositoryImpl이 Clean Architecture 원칙에 따라 구현되어 있습니다.
- 모든 리포지토리 메서드가 Flow를 활용하여 구현되어 있어 리액티브 프로그래밍 패턴을 잘 따르고 있습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# ReviewService와 관련 Repository 구현 확인
echo "Checking ReviewService implementation..."
ast-grep --pattern 'interface ReviewService {
$$$
}'
echo "Checking ReviewRepository implementation..."
ast-grep --pattern 'class ReviewRepositoryImpl $$$implements ReviewRepository {
$$$
}'
Length of output: 5181
Script:
#!/bin/bash
# ReviewRepository 인터페이스와 구현체 확인
echo "Checking ReviewRepository interface..."
rg -A 5 "interface ReviewRepository"
echo "Checking ReviewRepositoryImpl class..."
rg -A 10 "class ReviewRepositoryImpl"
# Flow 사용 여부 확인
echo "Checking Flow usage in repository..."
rg -A 3 "Flow<" "app/src/main/java/com/eatssu/android/data/repository"
Length of output: 15666
app/build.gradle.kts (2)
24-24
: 버전 코드가 적절하게 증가되었습니다.
메이저 리팩토링 작업에 따른 버전 코드 증가가 적절해 보입니다.
Line range hint 1-156
: Hilt와 Flow 관련 의존성이 올바르게 구성되어 있는지 확인해주세요.
현재 구성:
- Hilt 관련:
hilt
와hilt.compiler
의존성이 포함됨 - Flow 관련: Coroutines 의존성을 통해 지원됨
리팩토링 작업에 필요한 핵심 의존성들이 모두 포함되어 있습니다. 다만, 다음 사항들을 확인해주시기 바랍니다:
- Hilt 관련 테스트를 위한 의존성 추가 검토
- Flow 테스트를 위한
turbine
라이브러리 추가 검토
✅ Verification successful
Based on the shell script results, I can now provide a final response since we have comprehensive information about both Hilt and Flow usage in the codebase.
Hilt와 Flow 의존성이 적절하게 구성되어 있으며, 코드베이스 전반에 걸쳐 올바르게 사용되고 있습니다.
확인된 사항:
- Hilt 관련:
@HiltAndroidApp
,@AndroidEntryPoint
,@Inject
,@Module
,@InstallIn
등의 어노테이션이 전체적으로 올바르게 사용됨- 모든 ViewModel과 Repository에서 의존성 주입이 정상적으로 구현됨
- Flow 관련:
- StateFlow를 활용한 UI 상태 관리가 모든 ViewModel에서 일관되게 구현됨
- Repository 계층에서 Flow를 사용한 비동기 데이터 스트림 처리가 적절히 구현됨
추가적인 테스트 의존성은 현재 구현된 기능에 비해 충분한 수준입니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Hilt usage in the codebase
# Test: Search for Hilt annotations usage
echo "Checking Hilt annotations usage:"
rg "@HiltAndroidApp|@AndroidEntryPoint|@Inject|@Module|@InstallIn"
# Test: Search for Flow usage
echo "Checking Flow usage:"
rg "Flow<|StateFlow<|SharedFlow<"
Length of output: 21661
app/src/main/java/com/eatssu/android/ui/review/list/ReviewActivity.kt (2)
25-26
: ViewModel 초기화 구현이 개선되었습니다!
Hilt의 viewModels()
위임을 사용한 ViewModel 초기화는 보일러플레이트 코드를 줄이고 의존성 주입을 깔끔하게 처리합니다.
67-67
: 리뷰 로딩 로직이 개선되었습니다!
메뉴 타입에 따른 분기 처리를 ViewModel로 이동시켜 Activity의 책임을 줄이고 코드를 더 깔끔하게 만들었습니다.
app/src/main/res/values/strings.xml (1)
74-78
: 리뷰 수정 관련 문자열이 잘 구성되어 있습니다.
사용자 액션에 대한 피드백 메시지가 명확하고 일관성 있게 작성되어 있습니다.
app/src/main/java/com/eatssu/android/ui/review/list/ReviewViewModel.kt (3)
27-32
: 의존성 주입이 잘 구현되었습니다!
Clean Architecture 원칙에 따라 UseCase들이 적절하게 주입되었고, Hilt를 통한 의존성 주입이 올바르게 구현되었습니다.
38-59
: 이전 리뷰 코멘트가 여전히 유효합니다
메뉴 타입에 대한 문자열 리터럴 사용 관련 이전 리뷰 의견이 아직 반영되지 않았습니다.
Line range hint 227-235
: 상태 관리가 잘 구현되었습니다
ReviewState
데이터 클래스가 명확하게 구조화되어 있고, 상태 업데이트가 일관성 있게 처리되고 있습니다.
app/src/main/java/com/eatssu/android/ui/review/write/ReviewWriteRateActivity.kt (1)
342-346
: 상수 정의가 적절합니다.
REVIEW_MIN_LENGTH
와 PERMISSION_REQUEST_CODE
가 companion object에 잘 정의되어 있습니다.
app/src/main/java/com/eatssu/android/data/service/ReviewService.kt (5)
25-28
: 'writeReview' 메소드의 비동기 처리 적용 확인
suspend
키워드를 사용하여 writeReview
메소드를 비동기적으로 변경한 것을 확인했습니다. 이는 코루틴을 활용하여 네트워크 호출을 효율적으로 처리하는 데 도움이 됩니다.
31-33
: 'deleteReview' 메소드의 비동기 처리 적용 확인
suspend
키워드를 통해 deleteReview
메소드가 비동기적으로 변경되었습니다. 적절한 적용으로 판단됩니다.
36-39
: 'modifyReview' 메소드의 비동기 처리 적용 확인
modifyReview
메소드에 suspend
키워드를 추가하여 비동기 처리가 가능해졌습니다. 코루틴 기반의 네트워크 처리를 위해 필요한 수정입니다.
63-67
: 'uploadImage' 메소드 추가 및 네이밍 개선 확인
이미지 업로드를 위한 uploadImage
메소드가 추가된 것을 확인했습니다。기능에 맞는 적절한 메소드명을 사용하여 명확성을 높였습니다。
50-50
:
'sort' 파라미터의 전달 방식 확인 필요
@Query("sort") sort: List<String>? = arrayListOf("date", "DESC"),
에서 쿼리 파라미터로 List<String>
을 전달하는 방식이 서버에서 올바르게 처리되는지 확인이 필요합니다。일부 서버는 리스트 형태의 쿼리 파라미터를 제대로 처리하지 못할 수 있으므로,문자열로 변환하거나 적절한 형태로 전달하는 것을 고려해보세요。
서버와의 통신에서 문제가 없는지 확인하기 위해 테스트를 자동화할 수 있습니다。도움이 필요하시면 알려주세요。
suspend operator fun invoke(menuId: Long): Flow<BaseResponse<MenuOfMealResponse>> = | ||
mealRepository.getMenuInfoByMealId(menuId) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
에러 처리 로직 추가를 고려해주세요.
현재 구현은 깔끔하지만, 다음 사항들을 고려해보시면 좋겠습니다:
- 잘못된
mealId
값에 대한 유효성 검사 - Flow 스트림에서 발생할 수 있는 예외 처리
- 네트워크 오류나 기타 예외 상황에 대한 처리
예시 구현:
suspend operator fun invoke(mealId: Long): Flow<BaseResponse<MenuOfMealResponse>> {
require(mealId > 0) { "mealId는 0보다 커야 합니다" }
return mealRepository.getMenuInfoByMealId(mealId)
.catch { exception ->
emit(BaseResponse.Error(exception.message ?: "알 수 없는 오류가 발생했습니다"))
}
}
package com.eatssu.android.data.usecase.review | ||
|
||
import com.eatssu.android.base.BaseResponse | ||
import com.eatssu.android.data.dto.response.GetMenuReviewInfoResponse | ||
import com.eatssu.android.data.repository.ReviewRepository | ||
import kotlinx.coroutines.flow.Flow | ||
import javax.inject.Inject | ||
|
||
class GetMenuReviewInfoUseCase @Inject constructor( | ||
private val reviewRepository: ReviewRepository, | ||
) { | ||
suspend operator fun invoke(menuId: Long): Flow<BaseResponse<GetMenuReviewInfoResponse>> = | ||
reviewRepository.getMenuReviewInfo(menuId) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
UseCase 클래스의 위치가 아키텍처 구조와 일치하지 않습니다.
README.md에 명시된 Clean Architecture 패키지 구조에 따르면 UseCase는 domain/usecase
패키지에 위치해야 하지만, 현재 모든 UseCase가 data/usecase
패키지에 잘못 위치해 있습니다. 다음 변경이 필요합니다:
GetMenuReviewInfoUseCase.kt
를 포함한 모든 UseCase를app/src/main/java/com/eatssu/android/domain/usecase
패키지로 이동- Repository 의존성을 domain 레이어의 repository 인터페이스로 변경
🔗 Analysis chain
유스케이스 위치 검증이 필요합니다.
Clean Architecture에 따르면 UseCase는 domain 레이어에 위치해야 하는데, 현재 data 패키지에 위치해 있습니다.
아키텍처 레이어 구조를 확인하기 위해 다음 스크립트를 실행합니다:
Let me check if there are any architecture-related files like README or documentation that might provide insights about the intended architecture.
Let me check the README content and also look for any domain layer related files that might exist in different locations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: 프로젝트의 아키텍처 레이어 구조 확인
# domain 레이어 존재 여부 확인
fd -t d "domain$" app/src/main
# 다른 UseCase들의 위치 확인
fd -t f "UseCase.kt$" app/src/main
Length of output: 2241
Script:
#!/bin/bash
# Check for architecture documentation
fd -t f "README|ARCHITECTURE|architecture" -e md
# Check build.gradle files for module structure
fd -t f "build.gradle" -x cat {}
Length of output: 4722
Script:
#!/bin/bash
# Check README content
cat README.md
# Look for any domain layer related files
rg -g "!*.{kt,java}" "domain" -l
rg -g "*.{kt,java}" "package.*domain" --type-add "kt:*.kt" --type-add "java:*.java"
Length of output: 1416
} | ||
|
||
private fun onCheckBoxClick(position: Int) { | ||
val item = getItem(position) | ||
if (checkedItems.contains(item)) { | ||
checkedItems.remove(item) | ||
} else { | ||
checkedItems.add(item) | ||
checkedItems.add(item as Pair<String, Long>) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안전하지 않은 타입 캐스팅이 발견되었습니다.
item as Pair<String, Long>
는 런타임에 ClassCastException을 발생시킬 수 있습니다.
다음과 같이 수정하는 것을 제안드립니다:
- checkedItems.add(item as Pair<String, Long>)
+ if (item.first != null && item.second != null) {
+ checkedItems.add(Pair(item.first!!, item.second!!))
+ }
📝 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.
checkedItems.add(item as Pair<String, Long>) | |
if (item.first != null && item.second != null) { | |
checkedItems.add(Pair(item.first!!, item.second!!)) | |
} |
private fun onCheckBoxClick(position: Int) { | ||
val item = getItem(position) | ||
if (checkedItems.contains(item)) { | ||
checkedItems.remove(item) | ||
} else { | ||
checkedItems.add(item) | ||
checkedItems.add(item as Pair<String, Long>) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
onCheckBoxClick 메서드의 전반적인 로직 개선이 필요합니다.
현재 구현은 null 체크가 불충분하며, checkedItems와 비교 시 일관성이 없을 수 있습니다.
다음과 같은 리팩토링을 제안드립니다:
private fun onCheckBoxClick(position: Int) {
val item = getItem(position)
- if (checkedItems.contains(item)) {
- checkedItems.remove(item)
- } else {
- checkedItems.add(item as Pair<String, Long>)
- }
+ if (item.first != null && item.second != null) {
+ val nonNullItem = Pair(item.first!!, item.second!!)
+ if (checkedItems.contains(nonNullItem)) {
+ checkedItems.remove(nonNullItem)
+ } else {
+ checkedItems.add(nonNullItem)
+ }
+ }
}
Committable suggestion skipped: line range outside the PR's diff.
toastMessage = context.getString(R.string.delete_not) | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
문자열 리소스 접근 방식 개선이 필요합니다.
이전 리뷰에서 지적된 것처럼, ViewModel에서 직접 Context를 통한 리소스 접근은 권장되지 않습니다. 메모리 누수 위험이 있으며 테스트가 어려워집니다.
다음과 같은 개선 방안을 제안드립니다:
- ResourceProvider 클래스를 만들어 문자열 리소스 접근을 위임:
class ResourceProvider @Inject constructor(@ApplicationContext private val context: Context) {
fun getString(@StringRes resId: Int): String = context.getString(resId)
}
- ViewModel에서 ResourceProvider 사용:
- @ApplicationContext private val context: Context
+ private val resourceProvider: ResourceProvider
- toastMessage = context.getString(R.string.delete_not)
+ toastMessage = resourceProvider.getString(R.string.delete_not)
Also applies to: 52-53
it.copy( | ||
loading = false, | ||
error = true, | ||
toastMessage = "리뷰 작성에 실패하였습니다.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
문자열 리소스 외부화 필요
토스트 메시지가 코드에 직접 하드코딩되어 있습니다. 다국어 지원과 유지보수를 위해 strings.xml
로 분리하는 것이 좋습니다.
res/values/strings.xml
에 다음과 같이 추가하고 코드에서 참조하세요:
<resources>
<string name="review_upload_success">리뷰가 작성되었습니다.</string>
<string name="review_upload_failure">리뷰 작성에 실패하였습니다.</string>
</resources>
- toastMessage = "리뷰 작성에 실패하였습니다.",
+ toastMessage = context.getString(R.string.review_upload_failure),
- toastMessage = "리뷰가 작성되었습니다.",
+ toastMessage = context.getString(R.string.review_upload_success),
Also applies to: 75-75
// fun findMenuItemByMealId(mealId: Long) { | ||
// viewModelScope.launch { | ||
// mealService.getMenuInfoByMealId(mealId) | ||
// .enqueue(object : Callback<BaseResponse<MenuOfMealResponse>> { | ||
// override fun onResponse( | ||
// call: Call<BaseResponse<MenuOfMealResponse>>, | ||
// response: Response<BaseResponse<MenuOfMealResponse>>, | ||
// ) { | ||
// if (response.isSuccessful) { | ||
// val data = response.body()?.result | ||
// Log.d("post", "onResponse 성공" + response.body()) | ||
// _uiState.update { | ||
// it.copy( | ||
// menuOfMeal = response.body()?.result?.asMenuOfMeal() | ||
// ) | ||
// } | ||
// } else { | ||
// Log.d("post", "onResponse 실패") | ||
// } | ||
// } | ||
// | ||
// override fun onFailure( | ||
// call: Call<BaseResponse<MenuOfMealResponse>>, | ||
// t: Throwable, | ||
// ) { | ||
// Log.d("post", "onFailure 에러: ${t.message}") | ||
// } | ||
// }) | ||
// } | ||
// } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
주석 처리된 코드 제거 필요
findMenuItemByMealId
함수가 주석 처리되어 있습니다. 버전 관리 시스템(Git)을 사용하고 있으므로, 이런 주석 처리된 코드는 제거하는 것이 좋습니다.
주석 처리된 코드를 완전히 제거해주세요. 나중에 필요한 경우 Git 히스토리에서 확인할 수 있습니다.
} | ||
Timber.d("리뷰 있다") | ||
} | ||
|
||
override fun onFailure( | ||
call: Call<BaseResponse<GetMealReviewInfoResponse>>, | ||
t: Throwable, | ||
) { | ||
// 통신 실패 (인터넷 끊킴, 예외 발생 등 시스템적인 이유) | ||
Log.d("post", "onFailure 에러: " + t.message.toString()) | ||
} | ||
}) | ||
} | ||
} | ||
} | ||
} | ||
|
||
fun loadReviewList( | ||
menuType: MenuType, | ||
mealId: Long?, | ||
menuId: Long?, | ||
// lastReviewId: Long?, | ||
// page: Int?, | ||
// size: Int?, | ||
private fun callMenuReviewList( | ||
itemId: Long, | ||
) { | ||
viewModelScope.launch { | ||
|
||
reviewService.getReviewList(menuType.toString(), mealId, menuId) | ||
.enqueue(object : Callback<BaseResponse<GetReviewListResponse>> { | ||
override fun onResponse( | ||
call: Call<BaseResponse<GetReviewListResponse>>, | ||
response: Response<BaseResponse<GetReviewListResponse>>, | ||
) { | ||
if (response.isSuccessful) { | ||
Log.d("post", "onResponse 성공: " + response.body().toString()) | ||
|
||
val data = response.body()?.result!! | ||
|
||
if (data.numberOfElements == 0) { //리뷰 없어 시방 | ||
_uiState.update { | ||
it.copy( | ||
loading = false, | ||
error = false, | ||
isEmpty = true | ||
) | ||
} | ||
} else { //리뷰 있음 | ||
_uiState.update { | ||
it.copy( | ||
loading = false, | ||
error = false, | ||
reviewList = data.toReviewList(), | ||
isEmpty = false | ||
) | ||
} | ||
} | ||
// 정상적으로 통신이 성공된 경우 | ||
|
||
|
||
} else { | ||
// 통신이 실패한 경우(응답코드 3xx, 4xx 등) | ||
Log.d("post", "onResponse 실패") | ||
getMenuReviewListUseCase(itemId).onStart { | ||
_uiState.update { it.copy(loading = true) } | ||
}.onCompletion { | ||
_uiState.update { it.copy(loading = false, error = true) } | ||
}.catch { e -> | ||
_uiState.update { | ||
it.copy( | ||
loading = false, | ||
error = false, | ||
) | ||
} | ||
Timber.e(e.toString()) | ||
}.collectLatest { result -> | ||
result.result?.apply { | ||
if (numberOfElements == 0) { //리뷰 없음 | ||
_uiState.update { | ||
it.copy( | ||
loading = false, | ||
error = false, | ||
isEmpty = true | ||
) | ||
} | ||
} else { //리뷰 있음 | ||
_uiState.update { | ||
it.copy( | ||
loading = false, | ||
error = false, | ||
reviewList = this.toReviewList(), | ||
isEmpty = false | ||
) | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
코드 중복을 제거하여 유지보수성을 향상시킬 수 있습니다
리뷰 정보와 리스트를 가져오는 네 개의 메소드가 매우 유사한 패턴을 가지고 있습니다. 공통 로직을 추출하여 코드 중복을 제거하는 것이 좋겠습니다.
다음과 같은 방식으로 리팩토링을 제안합니다:
private suspend fun <T> executeUseCase(
useCase: suspend (Long) -> Flow<ApiResponse<T>>,
itemId: Long,
handleResult: (T) -> Unit
) {
useCase(itemId)
.onStart {
_uiState.update { it.copy(loading = true) }
}
.onCompletion {
_uiState.update { it.copy(loading = false, error = false) }
}
.catch { e ->
_uiState.update {
it.copy(
loading = false,
error = true
)
}
Timber.e(e)
}
.collectLatest { result ->
result.result?.let { handleResult(it) }
}
}
이렇게 하면 각 메소드는 결과 처리 로직만 전달하면 됩니다:
private fun callMenuReviewInfo(menuId: Long) {
viewModelScope.launch {
executeUseCase(
useCase = getMenuReviewInfoUseCase::invoke,
itemId = menuId
) { result ->
if (result.mainRating == null) {
_uiState.update {
it.copy(
reviewInfo = result.asReviewInfo(),
isEmpty = true
)
}
} else {
_uiState.update {
it.copy(
reviewInfo = result.asReviewInfo(),
isEmpty = false
)
}
}
}
}
}
Also applies to: 186-223
private var _imageFile: MutableStateFlow<File> = MutableStateFlow(File("")) | ||
val imageFile: StateFlow<File> get() = _imageFile |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
_imageFile을 nullable로 변경하고 초기화를 개선해주세요
현재 _imageFile
이 File("")
로 초기화되어 있습니다. 빈 문자열로 파일을 생성하면 예상치 못한 동작이 발생할 수 있습니다. _imageFile
을 nullable로 선언하고 초기 값을 null
로 설정하는 것을 고려해주세요. 이를 통해 파일이 존재하지 않을 경우를 명확하게 처리할 수 있습니다.
적용 가능한 변경 사항:
-private var _imageFile: MutableStateFlow<File> = MutableStateFlow(File(""))
+private var _imageFile: MutableStateFlow<File?> = MutableStateFlow(null)
또한, _imageFile.value
를 사용할 때 null 체크를 추가해야 합니다.
Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_imageFile.value를 사용할 때 null 체크를 어떻게 하면 좋을까?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_imageFile
이 nullable로 변경되었으므로, _imageFile.value
를 사용할 때 null 체크를 해주어야 합니다. Kotlin에서는 다음과 같은 방법으로 null 체크를 할 수 있습니다:
- if 문을 사용한 null 체크:
if (_imageFile.value != null) {
// _imageFile.value가 null이 아닐 때 실행할 코드
val file = _imageFile.value
// 파일과 관련된 작업 수행
} else {
// _imageFile.value가 null일 때 실행할 코드
}
- Safe Call 연산자
?.
와 Elvis 연산자?:
사용:
val fileExists = _imageFile.value?.exists() ?: false
if (fileExists) {
// 파일이 존재할 때 실행할 코드
} else {
// 파일이 존재하지 않을 때 실행할 코드
}
- let 함수를 사용한 null 체크:
_imageFile.value?.let { file ->
// file이 null이 아님을 보장받은 상태에서 실행할 코드
// 파일과 관련된 작업 수행
} ?: run {
// _imageFile.value가 null일 때 실행할 코드
}
- जब 확실하게 null이 아님을 알고 있다면, non-null 단언 연산자
!!
사용 (하지만 NullPointerException이 발생할 수 있으므로 주의):
val file = _imageFile.value!!
하지만 이 방법은 추천하지 않습니다.
위 방법들을 상황에 맞게 활용하시면 _imageFile.value
를 사용할 때 안전하게 null을 처리할 수 있습니다.
* add: usecase * feat: service&repository * refactor: 리뷰 삭제 리팩토링 * refactor: 리뷰 수정 리팩토링 * refactor: 토스트 string * refactor: service&repository * add: 리뷰 리스트&인포 usecase * refactor: suspend * add: 수정/삭제 string * add: 리뷰 요약/리스트 usecase * refactor: CA 리팩 * fix: 수정하기 됨 * refactor: 이미지 쪽 리팩 * refactor: 리뷰 작성 리팩 * refactor: 이미지 변환 content type 헤더 * delete: 이미지 서비스 합치기 * chore: 이미지 서비스 정리 * feat: 로그인 화면 finish() 추가 * refactor: timber * test: 이전 버전 * refactor: timber * refactor: imageusecase not working * chore: solved conflict * fix: not load meal's review * chore: pretter code * add: missed rebase * add: debug mode * chore: 리뷰 반영 * chore * package: change usecase folders * refactor: 다중 메뉴 선택 뷰 - 뷰모델 리팩토링 flow, repository, usecase * chore: @coderabbitai review * chore: @coderabbitai review2
* add: usecase * feat: service&repository * refactor: 리뷰 삭제 리팩토링 * refactor: 리뷰 수정 리팩토링 * refactor: 토스트 string * refactor: service&repository * add: 리뷰 리스트&인포 usecase * refactor: suspend * add: 수정/삭제 string * add: 리뷰 요약/리스트 usecase * refactor: CA 리팩 * fix: 수정하기 됨 * refactor: 이미지 쪽 리팩 * refactor: 리뷰 작성 리팩 * refactor: 이미지 변환 content type 헤더 * delete: 이미지 서비스 합치기 * chore: 이미지 서비스 정리 * feat: 로그인 화면 finish() 추가 * refactor: timber * test: 이전 버전 * refactor: timber * refactor: imageusecase not working * chore: solved conflict * fix: not load meal's review * chore: pretter code * add: missed rebase * add: debug mode * chore: 리뷰 반영 * chore * package: change usecase folders * refactor: 다중 메뉴 선택 뷰 - 뷰모델 리팩토링 flow, repository, usecase * chore: @coderabbitai review * chore: @coderabbitai review2
Summary
ReviewService를 중심으로, Review 도메인에 대하여 리팩토링을 실시합니다.
View -> Viewmodel (uiState) -> Usecase -> repository (flow)로 추상화 하였습니다.
Describe your changes
Issue
To reviewers
Summary by CodeRabbit
릴리스 노트
새로운 기능
버그 수정
문서화
리팩토링
테스트