Uh oh!
There was an error while loading. Please reload this page.
MainActivity companion object 안티패턴 제거 및 SharedFlow 이벤트 버스 전환 - #372
Merged
Conversation
…이벤트 버스로 전환 - ScreenRefreshEventBus 도입: Fragment 직접 참조 대신 SharedFlow 기반 이벤트 통신 - VisitorModeManager 도입: static mutable 변수 대신 Hilt Singleton으로 방문자 모드 관리 - MainActivity.companion에서 Fragment 인스턴스 참조, isVisitorMode, updateXxxScreen() 제거 - DiscoverFragment/StorageScrapFragment의 self-registration(onAttach/onDestroy) 제거 - Adapter에 isVisitorMode 람다 파라미터 전달로 정적 참조 제거
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughReplaces static MainActivity state with injected singletons: Changes
Sequence Diagram(s)sequenceDiagram
participant Upload as DiscoverUploadActivity
participant Bus as ScreenRefreshEventBus
participant Discover as DiscoverFragment
participant Scrap as StorageScrapFragment
rect rgba(200,200,255,0.5)
Upload->>Bus: emit(RefreshDiscoverCourses)
end
rect rgba(200,255,200,0.5)
Bus->>Discover: events -> RefreshDiscoverCourses
Discover->>Discover: refreshDiscoverCourses()
end
rect rgba(255,200,200,0.5)
Bus->>Scrap: events -> RefreshStorageScrap
Scrap->>Scrap: getMyScrapCourses()
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
app/src/main/java/com/runnect/runnect/presentation/event/VisitorModeManager.kt (1)
14-18: Optional: Consider caching login status for cleaner code.The
isVisitorModegetter performs a SharedPreferences read (getAccessToken()→ synchronizedPreferenceManager.getString()) on every invocation. In the current implementation, this is called only in click event listeners (DiscoverRecommendAdapterandDiscoverMarathonAdapter), so the frequency is low. However, caching the value at the call site can simplify the code and avoid repeated disk access:♻️ Option: Cache at call site
// In Fragment/Activity before creating adapter:val isVisitorMode = visitorModeManager.isVisitorMode adapter =DiscoverMultiViewAdapter( // ... isVisitorMode = { isVisitorMode }, // captured once// ... )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/runnect/runnect/presentation/event/VisitorModeManager.kt` around lines 14 - 18, The isVisitorMode getter calls context.getAccessToken() (which reads SharedPreferences) on every access; to avoid repeated disk reads and simplify call sites, capture visitorModeManager.isVisitorMode once where the adapters are created (e.g., in the Fragment/Activity before constructing DiscoverMultiViewAdapter/DiscoverRecommendAdapter/DiscoverMarathonAdapter) and pass that captured Boolean into the adapters (or pass a lambda that returns the captured value) instead of passing the live getter; this keeps the existing VisitorModeManager.isVisitorMode, reduces SharedPreferences accesses from LoginStatus.getLoginStatus(context.getAccessToken()), and makes adapter code cleaner.app/src/main/java/com/runnect/runnect/presentation/MainActivity.kt (1)
43-45: Remove the dead visitor-mode hook.
checkVisitorMode()no longer does any initialization, butonCreate()still calls it. Keeping the empty hook makes the startup path look stateful when it isn't.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/runnect/runnect/presentation/MainActivity.kt` around lines 43 - 45, Remove the dead hook by deleting the empty checkVisitorMode() function and its call from MainActivity.onCreate(), since VisitorModeManager now handles visitor-mode via Hilt; search for the checkVisitorMode() declaration and any invocation in MainActivity (including onCreate()) and remove both so startup path is not misleading, keeping any real initialization in VisitorModeManager instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/main/java/com/runnect/runnect/presentation/discover/DiscoverFragment.kt`:
- Around line 98-108: The refresh events are currently lost because
ScreenRefreshEventBus uses MutableSharedFlow with no replay; update the event
bus to buffer the last event (e.g., initialize
MutableSharedFlow<ScreenRefreshEvent>(replay = 1)) or replace it with a
StateFlow that holds the current refresh state so new collectors always see the
latest value; ensure this change is made in the ScreenRefreshEventBus
initialization (affecting collectors like
DiscoverFragment.collectScreenRefreshEvents and StorageScrapFragment) so
ScreenRefreshEvent.RefreshDiscoverCourses emitted while the view is destroyed
will be delivered when the fragment re-subscribes.
In
`@app/src/main/java/com/runnect/runnect/presentation/discover/upload/DiscoverUploadActivity.kt`:
- Around line 124-127: The event emission is happening after startActivity()
inside DiscoverUploadActivity and inside lifecycleScope.launch, which can be
cancelled when the Activity finishes and lost with MutableSharedFlow(replay=0);
move or duplicate the emit so it cannot be missed: emit
ScreenRefreshEvent.RefreshDiscoverCourses before calling startActivity() (or
change the flow to replay=1 in ScreenRefreshEventBus), or perform the emit from
a non-cancellable/app-scoped coroutine (e.g., an application-level scope or the
fragment/viewmodel scope) instead of lifecycleScope.launch so the refresh event
is delivered reliably; reference screenRefreshEventBus.emit, startActivity(),
lifecycleScope.launch, applyScreenExitAnimation, and
ScreenRefreshEventBus/MutableSharedFlow(replay=0) when applying the change.
In
`@app/src/main/java/com/runnect/runnect/presentation/event/ScreenRefreshEventBus.kt`:
- Line 16: The MutableSharedFlow _events in ScreenRefreshEventBus currently uses
the default replay=0 so emissions sent in
DiscoverUploadActivity.handleReturnToDiscover() can be missed by the target
Fragment; change the MutableSharedFlow construction in ScreenRefreshEventBus to
use replay = 1 so the latest event is available to late subscribers, and if you
need to avoid delivering stale events to future subscribers call
_events.resetReplayCache() after the consumer processes the event (refer to the
ScreenRefreshEventBus._events symbol and
DiscoverUploadActivity.handleReturnToDiscover() to locate the emit and
collection sites).
---
Nitpick comments:
In
`@app/src/main/java/com/runnect/runnect/presentation/event/VisitorModeManager.kt`:
- Around line 14-18: The isVisitorMode getter calls context.getAccessToken()
(which reads SharedPreferences) on every access; to avoid repeated disk reads
and simplify call sites, capture visitorModeManager.isVisitorMode once where the
adapters are created (e.g., in the Fragment/Activity before constructing
DiscoverMultiViewAdapter/DiscoverRecommendAdapter/DiscoverMarathonAdapter) and
pass that captured Boolean into the adapters (or pass a lambda that returns the
captured value) instead of passing the live getter; this keeps the existing
VisitorModeManager.isVisitorMode, reduces SharedPreferences accesses from
LoginStatus.getLoginStatus(context.getAccessToken()), and makes adapter code
cleaner.
In `@app/src/main/java/com/runnect/runnect/presentation/MainActivity.kt`:
- Around line 43-45: Remove the dead hook by deleting the empty
checkVisitorMode() function and its call from MainActivity.onCreate(), since
VisitorModeManager now handles visitor-mode via Hilt; search for the
checkVisitorMode() declaration and any invocation in MainActivity (including
onCreate()) and remove both so startup path is not misleading, keeping any real
initialization in VisitorModeManager instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 929d25d1-2022-44e6-8dac-80a9af9aa1bb
📒 Files selected for processing (15)
app/src/main/java/com/runnect/runnect/binding/BaseVisitorFragment.ktapp/src/main/java/com/runnect/runnect/presentation/MainActivity.ktapp/src/main/java/com/runnect/runnect/presentation/MainPager.ktapp/src/main/java/com/runnect/runnect/presentation/detail/CourseDetailActivity.ktapp/src/main/java/com/runnect/runnect/presentation/discover/DiscoverFragment.ktapp/src/main/java/com/runnect/runnect/presentation/discover/adapter/DiscoverMarathonAdapter.ktapp/src/main/java/com/runnect/runnect/presentation/discover/adapter/DiscoverRecommendAdapter.ktapp/src/main/java/com/runnect/runnect/presentation/discover/adapter/multiview/DiscoverMultiViewAdapter.ktapp/src/main/java/com/runnect/runnect/presentation/discover/adapter/multiview/DiscoverMultiViewHolder.ktapp/src/main/java/com/runnect/runnect/presentation/discover/adapter/multiview/DiscoverMultiViewHolderFactory.ktapp/src/main/java/com/runnect/runnect/presentation/discover/upload/DiscoverUploadActivity.ktapp/src/main/java/com/runnect/runnect/presentation/draw/DrawActivity.ktapp/src/main/java/com/runnect/runnect/presentation/event/ScreenRefreshEventBus.ktapp/src/main/java/com/runnect/runnect/presentation/event/VisitorModeManager.ktapp/src/main/java/com/runnect/runnect/presentation/storage/StorageScrapFragment.kt
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
unam98
commented
Apr 2, 2026
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Comment on lines
+15
to
+17
| class ScreenRefreshEventBus @Inject constructor() { | ||
| private val _events = MutableSharedFlow<ScreenRefreshEvent>() | ||
| val events: SharedFlow<ScreenRefreshEvent> = _events.asSharedFlow() |
CollaboratorAuthor
There was a problem hiding this comment.
기존엔 생명주기를 고려하지 않은 코드여서 안정적이지 않았음. 그래서 개선을 해내야 하는데 지금의 방식은 sharedFlow를 쓰면 이벤트를 쏘고나서 백그라운드 전환 등으로 이벤트가 소실됐을 때 복구가 안 돼서 문제가 생길 것 같은데 검토 필요해보임.
CollaboratorAuthor
There was a problem hiding this comment.
반영 완료 (1356ba1)
MutableSharedFlow(replay = 1)로 변경하여 마지막 이벤트를 버퍼링- late subscriber도 최신 이벤트를 수신 가능
- 이 앱에서 이벤트는 "화면 새로고침" 용도라 stale replay 시 최악의 경우 불필요한 리프레시 1회 — 실질적 부작용 없음
- SharedFlow 공식 문서 참고
- checkVisitorMode() dead code 삭제 - collectScreenRefreshEvents()를 addObserver() 안으로 이동 - MutableSharedFlow(replay=1)로 이벤트 소실 방지 - DiscoverUploadActivity에서 emit을 startActivity 전으로 이동
unam98
commented
Apr 2, 2026
CollaboratorAuthor
코드래빗 Nitpick 대응VisitorModeManager SharedPreferences 캐싱 제안 — 현재 코드 유지
|
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Apr 3, 2026
5 tasks
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
작업 배경
MainActivity.companion에 Fragment 인스턴스를 직접 보관하고,isVisitorMode를 static mutable 변수로 전역 공유하는 안티패턴이 존재변경 사항
ScreenRefreshEventBus.kt@SingletonSharedFlow 기반 화면 간 이벤트 디스패처VisitorModeManager.kt@Singleton방문자 모드 상태 관리 (토큰 기반 판단)MainActivity.ktisVisitorMode,updateXxxScreen()제거DiscoverFragment.ktStorageScrapFragment.ktMainPager.ktDrawActivity.ktVisitorModeManager주입으로 교체CourseDetailActivity.ktVisitorModeManager+ 이벤트 버스 주입DiscoverUploadActivity.ktDiscoverRecommendAdapter.ktisVisitorMode람다 파라미터로 변경DiscoverMarathonAdapter.ktisVisitorMode람다 파라미터로 변경BaseVisitorFragment.ktVisitorModeManager주입DiscoverMultiViewAdapter/Holder/FactoryisVisitorMode람다 전달 체인영향 범위
Test Plan
🤖 Generated with Claude Code
Summary by CodeRabbit