[Refactor/#192] DailyEmotion 데이터 흐름 개선#193
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough이벤트 기반 감정 갱신을 제거하고 StateFlow 기반 관찰로 전환했습니다. DailyEmotion에 fetchedDate/isStale가 추가되고, 저장소는 원격/로컬 분리, Mutex 동기화, 구독 시 자동 페치·캐시 로직을 구현합니다. ViewModel들은 ObserveDailyEmotionUseCase로 전환되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant VM as ViewModel
participant UC as ObserveDailyEmotionUseCase
participant Repo as EmotionRepositoryImpl
participant Remote as EmotionRemoteDataSource
participant Local as EmotionLocalDataSource
rect rgba(135,206,250,0.5)
VM->>UC: 구독 시작
end
UC->>Repo: observeDailyEmotion() 구독
Note right of Repo: onSubscription 트리거
Repo->>Repo: Mutex로 동시성 검사
alt 캐시가 없거나 isStale 또는 forceRefresh
Repo->>Remote: 원격 API 호출
Remote-->>Repo: 응답(감정 데이터)
Repo->>Local: saveDailyEmotion(DailyEmotion with fetchedDate)
Local-->>Repo: StateFlow 업데이트
Repo-->>UC: Flow로 Result 발행
UC-->>VM: UI 업데이트
else 캐시가 최신
Local-->>Repo: 현재 캐시값 반환
Repo-->>UC: Flow로 Result 발행 (네트워크 없음)
UC-->>VM: UI 업데이트
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
data/src/main/java/com/threegap/bitnagil/data/emotion/model/response/DailyEmotionResponse.kt (1)
21-28:⚠️ Potential issue | 🟠 Major
fetchedDate는 변환 시각이 아니라 요청 기준 일자를 사용해야 합니다.27번 줄에서
LocalDate.now()를 호출하면,EmotionRepositoryImpl.kt46번 줄에서 고정한 요청 일자와 어긋날 수 있습니다. 자정 전후에 응답이 돌아오면 전날 데이터를 받아놓고도fetchedDate가 오늘 날짜로 기록되어isStale이false가 되고, 다음 구독에서 재조회가 차단됩니다. 요청에 사용한LocalDate를toDomain(requestedDate: LocalDate)매개변수로 전달하는 방식이 안전합니다.수정 예시
-fun DailyEmotionResponse.toDomain(): DailyEmotion = +fun DailyEmotionResponse.toDomain(fetchedDate: LocalDate): DailyEmotion = DailyEmotion( type = emotionMarbleType, name = emotionMarbleName, imageUrl = imageUrl, homeMessage = emotionMarbleHomeMessage, - fetchedDate = LocalDate.now(), + fetchedDate = fetchedDate, )- val currentDate = LocalDate.now().toString() - emotionDataSource.fetchDailyEmotion(currentDate).map { - _dailyEmotionFlow.value = it.toDomain() + val requestedDate = LocalDate.now() + emotionDataSource.fetchDailyEmotion(requestedDate.toString()).map { + _dailyEmotionFlow.value = it.toDomain(requestedDate) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/src/main/java/com/threegap/bitnagil/data/emotion/model/response/DailyEmotionResponse.kt` around lines 21 - 28, DailyEmotionResponse.toDomain currently sets fetchedDate = LocalDate.now(), which can diverge from the request date used in EmotionRepositoryImpl and cause stale-check bugs; change the conversion to accept a requestedDate parameter (e.g., fun DailyEmotionResponse.toDomain(requestedDate: LocalDate)): construct DailyEmotion with fetchedDate = requestedDate, update all callers (notably where EmotionRepositoryImpl maps responses at the request site) to pass the same requestedDate used for the request, and ensure the DailyEmotion data class usage remains consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@data/src/main/java/com/threegap/bitnagil/data/emotion/repositoryImpl/EmotionRepositoryImpl.kt`:
- Around line 20-25: The current compareAndSet in isFetching causes a concurrent
fetch to short-circuit and return Result.success(Unit), which can drop a
subsequent refresh triggered by registerEmotion() or a subscription's initial
fetch; change the fetch control in fetchDailyEmotion() (the routine guarded by
isFetching.compareAndSet) so that when a fetch is already in progress it either
queues a single follow-up fetch or serializes requests: e.g., replace the
immediate success return with a flag/queue (e.g., pendingRefresh boolean) that
is set when a fetch is in progress and a new fetch is requested, and after the
in-flight fetch completes have the code check pendingRefresh and perform one
additional fetch if needed; update references to isFetching, compareAndSet,
fetchDailyEmotion(), and registerEmotion() to use this handshake so a trailing
refresh is not lost.
---
Outside diff comments:
In
`@data/src/main/java/com/threegap/bitnagil/data/emotion/model/response/DailyEmotionResponse.kt`:
- Around line 21-28: DailyEmotionResponse.toDomain currently sets fetchedDate =
LocalDate.now(), which can diverge from the request date used in
EmotionRepositoryImpl and cause stale-check bugs; change the conversion to
accept a requestedDate parameter (e.g., fun
DailyEmotionResponse.toDomain(requestedDate: LocalDate)): construct DailyEmotion
with fetchedDate = requestedDate, update all callers (notably where
EmotionRepositoryImpl maps responses at the request site) to pass the same
requestedDate used for the request, and ensure the DailyEmotion data class usage
remains consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 52f22984-cc82-4ed7-bda4-56d2b9b2db63
📒 Files selected for processing (9)
data/src/main/java/com/threegap/bitnagil/data/emotion/model/response/DailyEmotionResponse.ktdata/src/main/java/com/threegap/bitnagil/data/emotion/repositoryImpl/EmotionRepositoryImpl.ktdomain/src/main/java/com/threegap/bitnagil/domain/emotion/model/DailyEmotion.ktdomain/src/main/java/com/threegap/bitnagil/domain/emotion/model/EmotionChangeEvent.ktdomain/src/main/java/com/threegap/bitnagil/domain/emotion/repository/EmotionRepository.ktdomain/src/main/java/com/threegap/bitnagil/domain/emotion/usecase/GetEmotionChangeEventFlowUseCase.ktdomain/src/main/java/com/threegap/bitnagil/domain/emotion/usecase/ObserveDailyEmotionUseCase.ktpresentation/src/main/java/com/threegap/bitnagil/presentation/screen/home/HomeViewModel.ktpresentation/src/main/java/com/threegap/bitnagil/presentation/screen/recommendroutine/RecommendRoutineViewModel.kt
💤 Files with no reviewable changes (2)
- domain/src/main/java/com/threegap/bitnagil/domain/emotion/usecase/GetEmotionChangeEventFlowUseCase.kt
- domain/src/main/java/com/threegap/bitnagil/domain/emotion/model/EmotionChangeEvent.kt
data/src/main/java/com/threegap/bitnagil/data/emotion/repositoryImpl/EmotionRepositoryImpl.kt
Outdated
Show resolved
Hide resolved
l5x5l
reviewed
Mar 9, 2026
presentation/src/main/java/com/threegap/bitnagil/presentation/screen/home/HomeViewModel.kt
Show resolved
Hide resolved
l5x5l
reviewed
Mar 9, 2026
Contributor
l5x5l
left a comment
l5x5l
left a comment
There was a problem hiding this comment.
Event를 각 ViewModel에서 감지해 감정 데이터 조회를 호출하는 공통 로직을 제거한 것은 좋아 보입니다! 👍
다만 수정해주신 코드에서 DailyEmotionResponse의 toDomain이나 DailyEmotion의 isStale 같이 LocalDate를 사용하는 부분을 보면 해당 정보를 인자로 전달받아 사용하기 보다는 함수 내부에서 매번 LocalData.now()를 호출하고 있어 호출 시점마다 결과가 달라질 수 있는 비결정적 코드라는 느낌을 받았습니다...!
이 부분만 외부에서 LocalDate를 주입받아 사용할 수 있도록 변경해주시면 추후 테스트시 또는 기능 유지보수시에 더 좋을 것 같아요!
Member
Author
수정완료! -> 3edbfd1 |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
data/src/main/java/com/threegap/bitnagil/data/emotion/repositoryImpl/EmotionRepositoryImpl.kt (1)
20-25:⚠️ Potential issue | 🟠 Major진행 중 fetch를 성공으로 단락하면 최신 감정 갱신이 유실됩니다.
Line 44에서 이미 fetch 중이면 바로
Result.success(Unit)을 반환해서, Line 39의 후행 refresh가 Lines 23-25의 초기 구독 fetch와 겹칠 때 그대로 드롭됩니다. 첫 요청이 등록 이전 상태를 읽었다면_dailyEmotionFlow는 같은 날짜 동안 stale로 간주되지 않아 오래된 값을 유지할 수 있습니다. 중복 호출만 막지 말고, 진행 중 요청 뒤에 refresh 1회를 보장하도록 직렬화하거나 pending 플래그를 두는 쪽이 안전합니다.예시 수정안
-import java.util.concurrent.atomic.AtomicBoolean +import kotlinx.coroutines.sync.Mutex - private val isFetching = AtomicBoolean(false) + private val fetchMutex = Mutex() + `@Volatile` + private var pendingRefresh = false override suspend fun fetchDailyEmotion(): Result<Unit> { - if (!isFetching.compareAndSet(false, true)) return Result.success(Unit) + if (!fetchMutex.tryLock()) { + pendingRefresh = true + return Result.success(Unit) + } return try { - val today = LocalDate.now() - emotionDataSource.fetchDailyEmotion(today.toString()).map { - _dailyEmotionFlow.value = it.toDomain(today) - } + var result: Result<Unit> + do { + pendingRefresh = false + val today = LocalDate.now() + result = emotionDataSource.fetchDailyEmotion(today.toString()).map { + _dailyEmotionFlow.value = it.toDomain(today) + } + } while (pendingRefresh) + result } finally { - isFetching.set(false) + fetchMutex.unlock() } }Also applies to: 39-52
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/src/main/java/com/threegap/bitnagil/data/emotion/repositoryImpl/EmotionRepositoryImpl.kt` around lines 20 - 25, The current concurrency handling in EmotionRepositoryImpl uses isFetching to short-circuit fetchDailyEmotion and returns early, which can drop a later refresh requested from onSubscription and leave _dailyEmotionFlow stale; update fetchDailyEmotion (and related callers) so that when isFetching is true you record a pending-refresh flag (or enqueue one) instead of returning success immediately, and when the in-flight fetch completes the code checks that flag to run exactly one additional fetch (or emit a refresh) and clear it; reference isFetching, fetchDailyEmotion, _dailyEmotionFlow and dailyEmotionFlow to locate where to add the pending flag/serialization and ensure onSubscription-triggered fetches are not lost.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@data/src/main/java/com/threegap/bitnagil/data/emotion/repositoryImpl/EmotionRepositoryImpl.kt`:
- Around line 20-25: The current concurrency handling in EmotionRepositoryImpl
uses isFetching to short-circuit fetchDailyEmotion and returns early, which can
drop a later refresh requested from onSubscription and leave _dailyEmotionFlow
stale; update fetchDailyEmotion (and related callers) so that when isFetching is
true you record a pending-refresh flag (or enqueue one) instead of returning
success immediately, and when the in-flight fetch completes the code checks that
flag to run exactly one additional fetch (or emit a refresh) and clear it;
reference isFetching, fetchDailyEmotion, _dailyEmotionFlow and dailyEmotionFlow
to locate where to add the pending flag/serialization and ensure
onSubscription-triggered fetches are not lost.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b5f7366c-1d2e-4fa6-aeff-8c044aa64d10
📒 Files selected for processing (3)
data/src/main/java/com/threegap/bitnagil/data/emotion/model/response/DailyEmotionResponse.ktdata/src/main/java/com/threegap/bitnagil/data/emotion/repositoryImpl/EmotionRepositoryImpl.ktdomain/src/main/java/com/threegap/bitnagil/domain/emotion/model/DailyEmotion.kt
l5x5l
reviewed
Mar 11, 2026
data/src/main/java/com/threegap/bitnagil/data/emotion/repositoryImpl/EmotionRepositoryImpl.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
presentation/src/main/java/com/threegap/bitnagil/presentation/screen/recommendroutine/RecommendRoutineViewModel.kt (1)
32-37: 실패 시에도loadRecommendRoutines()가 호출됩니다
observeDailyEmotionUseCase()가Result.failure를 emit하면getOrNull()?.type이null을 반환하고, 이null값도distinctUntilChanged()를 통과하여loadRecommendRoutines()를 트리거합니다.의도된 동작이라면 문제없지만, 실패 시 불필요한 API 호출을 방지하려면 성공 케이스만 필터링하는 것을 고려해 보세요.
♻️ 실패 케이스 필터링 예시
repeatOnSubscription { observeDailyEmotionUseCase() + .filter { it.isSuccess } .map { result -> result.getOrNull()?.type } .distinctUntilChanged() .collect { loadRecommendRoutines() } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/main/java/com/threegap/bitnagil/presentation/screen/recommendroutine/RecommendRoutineViewModel.kt` around lines 32 - 37, The current flow in repeatOnSubscription uses observeDailyEmotionUseCase() and maps Result to result.getOrNull()?.type so a failure becomes null and still passes through distinctUntilChanged(), causing loadRecommendRoutines() to run on errors; update the chain to only forward successful emissions (e.g., filter for success or mapNotNull the Result to its type) before distinctUntilChanged() so that loadRecommendRoutines() is invoked only when a non-null/successful emotion type is emitted from observeDailyEmotionUseCase().data/src/main/java/com/threegap/bitnagil/data/emotion/datasource/EmotionLocalDataSource.kt (1)
3-9: 데이터 레이어에서 도메인 모델 직접 참조데이터 레이어(
EmotionLocalDataSource)가 도메인 레이어의DailyEmotion을 직접 import하고 있습니다. Clean Architecture 원칙상 데이터 레이어는 도메인 레이어에 의존하지 않는 것이 이상적이나, 로컬 캐시에서 별도 DTO 변환 오버헤드를 피하기 위한 실용적 선택으로 보입니다.현재 구조가 프로젝트 전반에서 일관되게 사용된다면 문제없습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/src/main/java/com/threegap/bitnagil/data/emotion/datasource/EmotionLocalDataSource.kt` around lines 3 - 9, EmotionLocalDataSource currently imports and exposes the domain model DailyEmotion, coupling the data layer to the domain; to fix this create a data-layer DTO/entity (e.g., DailyEmotionEntity) and change the EmotionLocalDataSource signature to use that DTO (replace val dailyEmotion: StateFlow<DailyEmotion?> and fun saveDailyEmotion(dailyEmotion: DailyEmotion) with the DTO types), then implement mapping functions between DailyEmotionEntity and com.threegap.bitnagil.domain.emotion.model.DailyEmotion in the repository or a mapper class so conversion happens at the boundary (keep method names saveDailyEmotion and clearCache but update their parameter/return types and add mappers).presentation/src/main/java/com/threegap/bitnagil/presentation/screen/home/HomeViewModel.kt (1)
59-72: 에러 핸들링이 누락되어 있습니다.
onFailure블록이 비어 있어 에러가 무시됩니다. 이 파일의 다른 에러 핸들러(Line 265, 282)는Log.e()로 로깅하고 있어 일관성이 떨어집니다. 디버깅을 위해 최소한 로깅을 추가하는 것이 좋습니다.♻️ 에러 로깅 추가 제안
private fun observeDailyEmotion() { intent { repeatOnSubscription { observeDailyEmotionUseCase().collect { result -> result.fold( onSuccess = { dailyEmotion -> reduce { state.copy(dailyEmotion = dailyEmotion.toUiModel()) } }, - onFailure = {}, + onFailure = { error -> + Log.e("HomeViewModel", "감정 정보 가져오기 실패: ${error.message}") + }, ) } } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/main/java/com/threegap/bitnagil/presentation/screen/home/HomeViewModel.kt` around lines 59 - 72, The onFailure branch in observeDailyEmotion() currently swallows errors; update the onFailure handler of observeDailyEmotionUseCase().collect to log the throwable (use Log.e with a clear tag/message similar to other handlers) and optionally update state or show an error UI if needed; locate the onFailure lambda in observeDailyEmotion() and replace the empty block with a Log.e(...) call referencing the throwable so failures aren't ignored.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@data/src/main/java/com/threegap/bitnagil/data/emotion/repositoryImpl/EmotionRepositoryImpl.kt`:
- Line 22: Rename the misspelled constructor parameter and all usages from
emotionLocalDateSource to emotionLocalDataSource in EmotionRepositoryImpl:
update the primary constructor parameter name and every reference inside methods
(e.g., calls currently referencing emotionLocalDateSource) to the correct
emotionLocalDataSource identifier so the class compiles and follows proper
naming.
---
Nitpick comments:
In
`@data/src/main/java/com/threegap/bitnagil/data/emotion/datasource/EmotionLocalDataSource.kt`:
- Around line 3-9: EmotionLocalDataSource currently imports and exposes the
domain model DailyEmotion, coupling the data layer to the domain; to fix this
create a data-layer DTO/entity (e.g., DailyEmotionEntity) and change the
EmotionLocalDataSource signature to use that DTO (replace val dailyEmotion:
StateFlow<DailyEmotion?> and fun saveDailyEmotion(dailyEmotion: DailyEmotion)
with the DTO types), then implement mapping functions between DailyEmotionEntity
and com.threegap.bitnagil.domain.emotion.model.DailyEmotion in the repository or
a mapper class so conversion happens at the boundary (keep method names
saveDailyEmotion and clearCache but update their parameter/return types and add
mappers).
In
`@presentation/src/main/java/com/threegap/bitnagil/presentation/screen/home/HomeViewModel.kt`:
- Around line 59-72: The onFailure branch in observeDailyEmotion() currently
swallows errors; update the onFailure handler of
observeDailyEmotionUseCase().collect to log the throwable (use Log.e with a
clear tag/message similar to other handlers) and optionally update state or show
an error UI if needed; locate the onFailure lambda in observeDailyEmotion() and
replace the empty block with a Log.e(...) call referencing the throwable so
failures aren't ignored.
In
`@presentation/src/main/java/com/threegap/bitnagil/presentation/screen/recommendroutine/RecommendRoutineViewModel.kt`:
- Around line 32-37: The current flow in repeatOnSubscription uses
observeDailyEmotionUseCase() and maps Result to result.getOrNull()?.type so a
failure becomes null and still passes through distinctUntilChanged(), causing
loadRecommendRoutines() to run on errors; update the chain to only forward
successful emissions (e.g., filter for success or mapNotNull the Result to its
type) before distinctUntilChanged() so that loadRecommendRoutines() is invoked
only when a non-null/successful emotion type is emitted from
observeDailyEmotionUseCase().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a32afea7-6919-4ad1-bfa5-8b2c6ecd8cd1
📒 Files selected for processing (10)
app/src/main/java/com/threegap/bitnagil/di/data/DataSourceModule.ktdata/src/main/java/com/threegap/bitnagil/data/emotion/datasource/EmotionLocalDataSource.ktdata/src/main/java/com/threegap/bitnagil/data/emotion/datasource/EmotionRemoteDataSource.ktdata/src/main/java/com/threegap/bitnagil/data/emotion/datasourceImpl/EmotionLocalDataSourceImpl.ktdata/src/main/java/com/threegap/bitnagil/data/emotion/datasourceImpl/EmotionRemoteDataSourceImpl.ktdata/src/main/java/com/threegap/bitnagil/data/emotion/repositoryImpl/EmotionRepositoryImpl.ktdomain/src/main/java/com/threegap/bitnagil/domain/emotion/repository/EmotionRepository.ktdomain/src/main/java/com/threegap/bitnagil/domain/emotion/usecase/ObserveDailyEmotionUseCase.ktpresentation/src/main/java/com/threegap/bitnagil/presentation/screen/home/HomeViewModel.ktpresentation/src/main/java/com/threegap/bitnagil/presentation/screen/recommendroutine/RecommendRoutineViewModel.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- domain/src/main/java/com/threegap/bitnagil/domain/emotion/repository/EmotionRepository.kt
data/src/main/java/com/threegap/bitnagil/data/emotion/repositoryImpl/EmotionRepositoryImpl.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
🧹 Nitpick comments (1)
data/src/main/java/com/threegap/bitnagil/data/emotion/repositoryImpl/EmotionRepositoryImpl.kt (1)
79-81:toDomain(today)중복 호출 개선 필요
onSuccess와map에서 각각toDomain(today)를 호출하여 동일한 변환이 두 번 수행됩니다. 한 번만 변환하여 재사용하는 것이 효율적입니다.♻️ 개선 제안
- emotionRemoteDataSource.fetchDailyEmotion(today.toString()) - .onSuccess { emotionLocalDataSource.saveDailyEmotion(it.toDomain(today)) } - .map { it.toDomain(today) } + emotionRemoteDataSource.fetchDailyEmotion(today.toString()) + .map { it.toDomain(today) } + .onSuccess { emotionLocalDataSource.saveDailyEmotion(it) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/src/main/java/com/threegap/bitnagil/data/emotion/repositoryImpl/EmotionRepositoryImpl.kt` around lines 79 - 81, The code calls toDomain(today) twice when handling emotionRemoteDataSource.fetchDailyEmotion; refactor to compute the domain object once, reuse it for saving and mapping. Specifically, inside the fetchDailyEmotion chain (the call using onSuccess and map), transform the fetched DTO to a single val domain = it.toDomain(today) (or use let/also) and pass that same domain to emotionLocalDataSource.saveDailyEmotion(domain) and to the downstream mapping instead of calling toDomain(today) again.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@data/src/main/java/com/threegap/bitnagil/data/emotion/repositoryImpl/EmotionRepositoryImpl.kt`:
- Around line 79-81: The code calls toDomain(today) twice when handling
emotionRemoteDataSource.fetchDailyEmotion; refactor to compute the domain object
once, reuse it for saving and mapping. Specifically, inside the
fetchDailyEmotion chain (the call using onSuccess and map), transform the
fetched DTO to a single val domain = it.toDomain(today) (or use let/also) and
pass that same domain to emotionLocalDataSource.saveDailyEmotion(domain) and to
the downstream mapping instead of calling toDomain(today) again.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 61e53630-9888-436b-bba4-caca5e28bc80
📒 Files selected for processing (1)
data/src/main/java/com/threegap/bitnagil/data/emotion/repositoryImpl/EmotionRepositoryImpl.kt
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
presentation/src/main/java/com/threegap/bitnagil/presentation/screen/home/HomeViewModel.kt (1)
59-72:observeDailyEmotion의 실패 처리가 누락되었습니다.
onFailure = {}로 빈 처리를 하면 감정 데이터 로드 실패 시 사용자에게 피드백이 없습니다. 에러 로깅 또는 UI 상태 반영을 고려해주세요.♻️ 제안 수정
onFailure = { + Log.e("HomeViewModel", "감정 데이터 가져오기 실패: ${it.message}") },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/main/java/com/threegap/bitnagil/presentation/screen/home/HomeViewModel.kt` around lines 59 - 72, The observeDailyEmotion() handler currently ignores failures (onFailure = {}) so add real failure handling: capture the throwable from observeDailyEmotionUseCase()’s Result in the onFailure lambda, log the error (use your logger) and update ViewModel state via reduce(...) to reflect the error (e.g., set an error message flag or clear loading in state.copy(...) and/or set dailyEmotion to an error UI model). Use the existing symbols observeDailyEmotion(), observeDailyEmotionUseCase(), onFailure, reduce and state.copy(...) to locate where to insert logging and a state update so the UI can show feedback when loading the daily emotion fails.presentation/src/main/java/com/threegap/bitnagil/presentation/screen/mypage/MyPageViewModel.kt (1)
29-29: 실패 시 처리 로직이 누락되었습니다.
onFailure = {}로 빈 처리를 하면 네트워크 오류 시 사용자에게 아무런 피드백이 없습니다. 최소한 로그를 남기거나 에러 상태를 UI에 반영하는 것을 권장합니다.♻️ 제안 수정
onFailure = { + android.util.Log.e("MyPageViewModel", "유저 정보 가져오기 실패: ${it.message}") },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/main/java/com/threegap/bitnagil/presentation/screen/mypage/MyPageViewModel.kt` at line 29, The empty onFailure = {} in MyPageViewModel provides no feedback on network errors; replace it with a handler that logs the throwable and surfaces an error state to the UI (e.g., update a MutableLiveData/StateFlow or set a field on your existing _uiState) so the UI can show an error message; locate the network call inside MyPageViewModel where onFailure is passed and implement logging (use your logger) and state update (e.g., _error.postValue or _uiState.update { it.copy(error = ... ) }) in that onFailure lambda.data/src/main/java/com/threegap/bitnagil/data/user/repositoryImpl/UserRepositoryImpl.kt (1)
52-56:toDomain()이 두 번 호출됩니다 (최적화 제안).성공 시
onSuccess블록에서response.toDomain()을 호출하여 캐시에 저장하고, 이후map { it.toDomain() }에서 다시 변환을 수행합니다. 기능상 문제는 없지만 불필요한 객체 생성이 발생합니다.♻️ 제안 수정
userRemoteDataSource.fetchUserProfile() + .map { it.toDomain() } .onSuccess { response -> - userLocalDataSource.saveUserProfile(response.toDomain()) + userLocalDataSource.saveUserProfile(response) } - .map { it.toDomain() } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/src/main/java/com/threegap/bitnagil/data/user/repositoryImpl/UserRepositoryImpl.kt` around lines 52 - 56, The code calls response.toDomain() twice (once in onSuccess and again in map); fix by converting to domain once and reusing it: in the chain, replace the separate onSuccess + map with a single transformation that does val domain = response.toDomain(); userLocalDataSource.saveUserProfile(domain); return domain — i.e., inside the mapping lambda (for userRemoteDataSource.fetchUserProfile()), compute domain once, call userLocalDataSource.saveUserProfile(domain), and emit domain, removing the redundant toDomain() call in onSuccess/map.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@data/src/main/java/com/threegap/bitnagil/data/user/datasource/UserLocalDataSource.kt`:
- Line 8: Remove the unnecessary suspend modifier from
UserLocalDataSource.saveUserProfile: the implementation only calls
_userProfile.update { userProfile } (no suspend points), so change the signature
to a regular function to match EmotionLocalDataSource.saveDailyEmotion() and
clearCache() and maintain API consistency; update any call sites if they treat
it as suspend (remove coroutine/suspend usage) and ensure the method remains
non-blocking.
In
`@presentation/src/main/java/com/threegap/bitnagil/presentation/screen/home/HomeViewModel.kt`:
- Around line 257-274: The observeUserProfile() handler increments loadingCount
once at subscription but decrements it on every emission, which can drive
loadingCount negative; modify observeUserProfile() so it only decrements
loadingCount once (e.g., on the first successful or failed emission) by tracking
a local flag or using an operator that emits only the first emission for the
decrement logic; ensure observeUserProfileUseCase() collections still update
userNickname on every emission but only the first emission triggers reduce {
state.copy(loadingCount = state.loadingCount - 1) } while subsequent emissions
skip the decrement.
In
`@presentation/src/main/java/com/threegap/bitnagil/presentation/screen/mypage/MyPageViewModel.kt`:
- Line 27: reduce 블록에서 state.copy(name = it.nickname, profileUrl =
"profileUrl")처럼 profileUrl을 하드코딩하고 있으므로 UserProfile/데이터 소스에서 실제 값을 사용하도록 수정하세요:
확인할 대상은 UserProfile 모델(필드명: profileUrl)과 reduce 내부에서 전달되는 객체(it)의 프로파일 URL
필드명(예: it.profileUrl 또는 it.avatarUrl 등); 가능한 fixes는 UserProfile에 profileUrl 필드
추가 후 state.copy(name = it.nickname, profileUrl = it.profileUrl)로 변경하거나, 실제 프로필
URL을 반환하는 다른 소스에서 값을 받아 state.copy에 전달하거나 당장이라면 임시로 빈 문자열 state.copy(name =
it.nickname, profileUrl = "")로 변경하는 것입니다.
---
Nitpick comments:
In
`@data/src/main/java/com/threegap/bitnagil/data/user/repositoryImpl/UserRepositoryImpl.kt`:
- Around line 52-56: The code calls response.toDomain() twice (once in onSuccess
and again in map); fix by converting to domain once and reusing it: in the
chain, replace the separate onSuccess + map with a single transformation that
does val domain = response.toDomain();
userLocalDataSource.saveUserProfile(domain); return domain — i.e., inside the
mapping lambda (for userRemoteDataSource.fetchUserProfile()), compute domain
once, call userLocalDataSource.saveUserProfile(domain), and emit domain,
removing the redundant toDomain() call in onSuccess/map.
In
`@presentation/src/main/java/com/threegap/bitnagil/presentation/screen/home/HomeViewModel.kt`:
- Around line 59-72: The observeDailyEmotion() handler currently ignores
failures (onFailure = {}) so add real failure handling: capture the throwable
from observeDailyEmotionUseCase()’s Result in the onFailure lambda, log the
error (use your logger) and update ViewModel state via reduce(...) to reflect
the error (e.g., set an error message flag or clear loading in state.copy(...)
and/or set dailyEmotion to an error UI model). Use the existing symbols
observeDailyEmotion(), observeDailyEmotionUseCase(), onFailure, reduce and
state.copy(...) to locate where to insert logging and a state update so the UI
can show feedback when loading the daily emotion fails.
In
`@presentation/src/main/java/com/threegap/bitnagil/presentation/screen/mypage/MyPageViewModel.kt`:
- Line 29: The empty onFailure = {} in MyPageViewModel provides no feedback on
network errors; replace it with a handler that logs the throwable and surfaces
an error state to the UI (e.g., update a MutableLiveData/StateFlow or set a
field on your existing _uiState) so the UI can show an error message; locate the
network call inside MyPageViewModel where onFailure is passed and implement
logging (use your logger) and state update (e.g., _error.postValue or
_uiState.update { it.copy(error = ... ) }) in that onFailure lambda.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a584fe50-b268-4049-9992-47835f28f2cc
📒 Files selected for processing (16)
app/src/main/java/com/threegap/bitnagil/di/data/DataSourceModule.ktdata/build.gradle.ktsdata/src/main/java/com/threegap/bitnagil/data/user/datasource/UserLocalDataSource.ktdata/src/main/java/com/threegap/bitnagil/data/user/datasource/UserRemoteDataSource.ktdata/src/main/java/com/threegap/bitnagil/data/user/datasourceImpl/UserLocalDataSourceImpl.ktdata/src/main/java/com/threegap/bitnagil/data/user/datasourceImpl/UserRemoteDataSourceImpl.ktdata/src/main/java/com/threegap/bitnagil/data/user/repositoryImpl/UserRepositoryImpl.ktdata/src/test/java/com/threegap/bitnagil/data/user/repositoryImpl/UserRepositoryImplTest.ktdomain/src/main/java/com/threegap/bitnagil/domain/auth/usecase/LogoutUseCase.ktdomain/src/main/java/com/threegap/bitnagil/domain/auth/usecase/WithdrawalUseCase.ktdomain/src/main/java/com/threegap/bitnagil/domain/user/repository/UserRepository.ktdomain/src/main/java/com/threegap/bitnagil/domain/user/usecase/GetUserProfileUseCase.ktdomain/src/main/java/com/threegap/bitnagil/domain/user/usecase/ObserveUserProfileUseCase.ktpresentation/src/main/java/com/threegap/bitnagil/presentation/screen/home/HomeViewModel.ktpresentation/src/main/java/com/threegap/bitnagil/presentation/screen/mypage/MyPageViewModel.ktpresentation/src/main/java/com/threegap/bitnagil/presentation/screen/onboarding/OnBoardingViewModel.kt
data/src/main/java/com/threegap/bitnagil/data/user/datasource/UserLocalDataSource.kt
Outdated
Show resolved
Hide resolved
presentation/src/main/java/com/threegap/bitnagil/presentation/screen/home/HomeViewModel.kt
Show resolved
Hide resolved
presentation/src/main/java/com/threegap/bitnagil/presentation/screen/mypage/MyPageViewModel.kt
Show resolved
Hide resolved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
[ PR Content ]
스크럼 시간에 언급했던 flow 기반의 데이터 스트림 방식을 DailyEmotion(오늘의 감정) 부분에 먼저 적용을 해봤습니다.
작업 문서는 노션에 띠로 정리했습니다.! -> 🔗 노션 링크
Related issue
Screenshot 📸
Work Description
To Reviewers 📢
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선 사항
제거