Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package com.runnect.runnect.presentation.base

import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import kotlinx.coroutines.CoroutineExceptionHandler
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.SharedFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asSharedFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.catch
import kotlinx.coroutines.flow.onStart
import kotlinx.coroutines.launch
import retrofit2.HttpException
import timber.log.Timber
import java.net.SocketException
import java.net.UnknownHostException

abstract class MviViewModel<STATE, INTENT, EFFECT>(
Comment thread
unam98 marked this conversation as resolved.
Outdated
initialState: STATE
) : ViewModel() {

private val _state = MutableStateFlow(initialState)
val state: StateFlow<STATE> = _state.asStateFlow()

private val _effect = MutableSharedFlow<EFFECT>()
val effect: SharedFlow<EFFECT> = _effect.asSharedFlow()
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In Kotlin Coroutines, what happens to values emitted to a default MutableSharedFlow() before any collector is active, and what pattern is recommended for one-off UI effects?

💡 Result:

In a default MutableSharedFlow (replay=0, extraBufferCapacity=0, onBufferOverflow=BufferOverflow.SUSPEND), values emitted before any collector is active are dropped/lost. The emit call returns immediately without suspending, as there is no buffer and no subscribers; the value is simply not stored. For one-off UI effects (e.g., toasts, navigation), the recommended pattern is to use MutableSharedFlow with replay=0 (to avoid replaying to new collectors after configuration changes) exposed as SharedFlow, collected in UI with lifecycle-aware scopes like repeatOnLifecycle(Lifecycle.State.STARTED) or LaunchedEffect. Some sources suggest Channel + receiveAsFlow for guaranteed delivery via buffering, but official docs and common practice favor SharedFlow configured with replay=0 for events.

Citations:


🏁 Script executed:

# Get the file path and read relevant sections
fd -t f "MviViewModel.kt" --path "*/presentation/base/*"

Repository: Runnect/Runnect-Android

Length of output: 300


🏁 Script executed:

# Read the MviViewModel.kt file to understand context around lines 28-29 and 50-53
cat -n app/src/main/java/com/runnect/runnect/presentation/base/MviViewModel.kt | sed -n '1,70p'

Repository: Runnect/Runnect-Android

Length of output: 2735


🏁 Script executed:

# Search for where effect is collected in the codebase to see if lifecycle-aware collection is used
rg "\.effect\b" --type kotlin -A 3 -B 3 | head -100

Repository: Runnect/Runnect-Android

Length of output: 49


🏁 Script executed:

# Search for effect collection pattern in the codebase
rg "\.effect\s*\." --type kotlin -B 2 -A 5

Repository: Runnect/Runnect-Android

Length of output: 49


🏁 Script executed:

# Search for LaunchedEffect or repeatOnLifecycle patterns with effect
rg "LaunchedEffect|repeatOnLifecycle" --type kotlin -B 2 -A 5 | head -80

Repository: Runnect/Runnect-Android

Length of output: 2099


🏁 Script executed:

# Look for specific MyPage files mentioned in the comment to see how effects are collected
fd "MyPage" --type f -e kt -e java | head -20

Repository: Runnect/Runnect-Android

Length of output: 482


🏁 Script executed:

# Check MyPageFragment to see how effect is collected
cat -n app/src/main/java/com/runnect/runnect/presentation/mypage/MyPageFragment.kt | head -150

Repository: Runnect/Runnect-Android

Length of output: 7228


🏁 Script executed:

# Check MyPageViewModel to see what effects it posts
cat -n app/src/main/java/com/runnect/runnect/presentation/mypage/MyPageViewModel.kt | head -100

Repository: Runnect/Runnect-Android

Length of output: 2166


🏁 Script executed:

# Get the full MyPageFragment to see if effect is collected
cat -n app/src/main/java/com/runnect/runnect/presentation/mypage/MyPageFragment.kt | tail -100

Repository: Runnect/Runnect-Android

Length of output: 4430


🏁 Script executed:

# Search for any effect collection in MyPageFragment
rg "viewModel\.effect" app/src/main/java/com/runnect/runnect/presentation/mypage/MyPageFragment.kt -A 5

Repository: Runnect/Runnect-Android

Length of output: 49


🏁 Script executed:

# Check MyPageContract to see what effects are defined
cat app/src/main/java/com/runnect/runnect/presentation/mypage/MyPageContract.kt

Repository: Runnect/Runnect-Android

Length of output: 832


Collect effects in the UI layer with lifecycle awareness.

The effect flow is defined but never collected in MyPageFragment. Effects like MyPageEffect.ShowError are posted but will be lost if no collector is active. Add effect collection in the Fragment using the existing repeatOnStarted pattern: repeatOnStarted { viewModel.effect.collect { effect -> handleEffect(effect) } }. A default MutableSharedFlow() is appropriate for MVI when effects are properly collected with lifecycle-aware scopes; no need for Channel.

Also applies to: 50-53

🤖 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/base/MviViewModel.kt`
around lines 28 - 29, The effect SharedFlow (_effect: MutableSharedFlow<EFFECT>
and public effect: SharedFlow<EFFECT>) is never collected in the UI so posted
effects like MyPageEffect.ShowError are dropped; in the Fragment that owns
MyPageViewModel (e.g., MyPageFragment) add a lifecycle-aware collector using the
existing repeatOnStarted pattern to collect viewModel.effect and dispatch each
emission to your effect handler (e.g., call handleEffect(effect) or the existing
effect routing method); keep the MutableSharedFlow as-is and ensure collection
is started in onViewCreated (or equivalent) so effects are received only while
the UI is active.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

현재 코드 유지

  • postEffect()viewModelScope.launch에서 emit()하므로 ViewModel이 살아있는 한 emit 자체는 실행됨
  • Fragment는 repeatOnStarted로 collect하므로, Fragment가 STARTED 이상일 때만 수신 — lifecycle-safe
  • extraBufferCapacity = 1은 effect가 "fire-and-forget" 성격일 때 적합하지만, 현재 effect가 에러 토스트뿐이고 이미 state.error로도 에러를 보여주고 있어 effect 미수신 시 실질적 문제 없음
  • SharedFlow 공식 문서: replay=0, extraBufferCapacity=0이 default이며, 이는 collector가 없으면 emit이 suspend되는 동작 — drop이 아님
  • 추후 effect 종류가 늘어나면 그때 extraBufferCapacity 추가 검토

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@unam98, 결정 존중합니다. 그리고 공식 문서 인용을 통한 중요한 정정 감사합니다.

제 원래 코멘트에서 "dropped"라고 표현한 것은 부정확했습니다. MutableSharedFlow의 기본 emit()은 collector가 없을 때 suspend 동작이며 (drop은 tryEmit()의 동작), viewModelScope.launch { _effect.emit(effect) }로 실행되므로 Fragment가 STARTED 상태가 되어 collect를 시작하면 해당 emission을 수신할 수 있습니다. 현재 구조에서 충분히 합리적입니다.

( ˘▾˘)~♪


✏️ Learnings added
Learnt from: unam98
Repo: Runnect/Runnect-Android PR: 373
File: app/src/main/java/com/runnect/runnect/presentation/base/MviViewModel.kt:25-26
Timestamp: 2026-04-02T17:59:43.992Z
Learning: In the MVI base class `MviViewModel.kt` (`app/src/main/java/com/runnect/runnect/presentation/base/MviViewModel.kt`), `postEffect()` uses `viewModelScope.launch { _effect.emit(effect) }` with the default `MutableSharedFlow(replay=0, extraBufferCapacity=0)`. The team intentionally keeps this configuration because: (1) `emit()` suspends (not drops) when no collector is active, (2) the Fragment collects with `repeatOnStarted` for lifecycle safety, and (3) current effects (ShowError) are also mirrored in `state.error`, making missed effects non-critical. `extraBufferCapacity` will be revisited if more effect types are added.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


val currentState: STATE get() = _state.value

private val exceptionHandler = CoroutineExceptionHandler { _, throwable ->
Timber.tag(throwable::class.java.simpleName).e(throwable)
handleException(throwable)
}

fun intent(intent: INTENT) {
viewModelScope.launch(exceptionHandler) {
handleIntent(intent)
}
}

protected abstract suspend fun handleIntent(intent: INTENT)

protected fun reduce(reducer: STATE.() -> STATE) {
_state.value = currentState.reducer()
}

protected fun postEffect(effect: EFFECT) {
viewModelScope.launch {
_effect.emit(effect)
}
}

protected fun <T> collectFlow(
flow: suspend () -> Flow<Result<T>>,
onLoading: () -> Unit = {},
onSuccess: (T) -> Unit,
onFailure: (Throwable) -> Unit
) {
viewModelScope.launch(exceptionHandler) {
flow()
.onStart { onLoading() }
.catch { onFailure(it) }
.collect { result ->
result.fold(onSuccess, onFailure)
}
}
}

protected open fun handleException(throwable: Throwable) {
when (throwable) {
is SocketException,
is HttpException,
is UnknownHostException -> Timber.e(throwable)
else -> Timber.e(throwable)
Comment thread
unam98 marked this conversation as resolved.
Outdated
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package com.runnect.runnect.presentation.mypage

import com.runnect.runnect.R

data class MyPageUiState(
val isLoading: Boolean = true,
val nickname: String = "",
val stampId: String = STAMP_LOCK,
val profileImgResId: Int = R.drawable.user_profile_basic,
val level: String = "",
val levelPercent: Int = 0,
val email: String = "",
val error: String? = null
) {
companion object {
const val STAMP_LOCK = "lock"
}
Comment on lines +15 to +17
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기서 단순히 매직 리터럴을 없애겠단 이유 말고도 companion object를 쓴 이유가 있는지? 메모리 효율에 대한 고려는 했는지?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

companion object 안의 const val은 컴파일 타임에 인라인되어 메모리에 별도 객체를 만들지 않습니다.

  • const val STAMP_LOCK = "lock"은 바이트코드에서 사용처에 직접 "lock" 리터럴로 치환됨
  • 일반 val이었다면 companion object 인스턴스가 힙에 올라가겠지만, const는 그렇지 않음
  • Kotlin const val 공식 문서

여기서 companion을 쓴 이유는 단순히 data class 바깥에 상수를 두면서도 MyPageUiState.STAMP_LOCK으로 접근할 수 있게 스코핑한 것입니다.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

팩트 체크는 안 해봤는데 너가 한 말에 따르면 아래와 같이 이해했는데 맞아?

  • val : 일반적으로 스택에 쌓임
  • companion object 안의 val : 싱글턴이라 힙에 쌓임
  • companion object 안의 const val : 메모리 할당 안 됨

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

거의 맞는데 하나만 정정:

  • val (로컬 변수): 스택에 할당 ✅
  • val (클래스 프로퍼티): 에 할당 (객체의 일부이므로)
  • companion object 안의 val: companion object 인스턴스가 에 올라가고, 그 안의 val도 힙 ✅
  • companion object 안의 const val: 컴파일 타임에 사용처에 인라인 → 런타임 메모리 할당 없음 ✅

정리하면 val의 위치(로컬 vs 클래스 프로퍼티)에 따라 스택/힙이 달라지고, const val만 인라인되어 메모리 할당이 없습니다.

참고: Kotlin const 공식 문서

}

sealed interface MyPageIntent {
data object LoadUserInfo : MyPageIntent
data class UpdateNickname(val nickname: String) : MyPageIntent
data class UpdateProfileImg(val resId: Int) : MyPageIntent
}

sealed interface MyPageEffect {
data class ShowError(val message: String) : MyPageEffect
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import androidx.core.view.isVisible
import androidx.fragment.app.activityViewModels
import androidx.fragment.app.commit
import androidx.fragment.app.replace
import coil3.load
import com.kakao.sdk.common.util.KakaoCustomTabsClient
import com.kakao.sdk.talk.TalkApiClient
import com.runnect.runnect.BuildConfig
Expand All @@ -21,13 +22,14 @@ import com.runnect.runnect.presentation.mypage.history.MyHistoryActivity
import com.runnect.runnect.presentation.mypage.reward.MyRewardActivity
import com.runnect.runnect.presentation.mypage.setting.MySettingFragment
import com.runnect.runnect.presentation.mypage.upload.MyUploadActivity
import com.runnect.runnect.presentation.state.UiState
import com.runnect.runnect.util.analytics.Analytics
import com.runnect.runnect.util.analytics.EventName.EVENT_CLICK_GOAL_REWARD
import com.runnect.runnect.util.analytics.EventName.EVENT_CLICK_RUNNING_RECORD
import com.runnect.runnect.util.analytics.EventName.EVENT_CLICK_UPLOADED_COURSE
import com.runnect.runnect.util.extension.getStampResId
import com.runnect.runnect.util.extension.repeatOnStarted
import dagger.hilt.android.AndroidEntryPoint
import kotlinx.coroutines.flow.collectLatest

@AndroidEntryPoint
class MyPageFragment : BaseVisitorFragment<FragmentMyPageBinding>(R.layout.fragment_my_page) {
Expand All @@ -38,9 +40,8 @@ class MyPageFragment : BaseVisitorFragment<FragmentMyPageBinding>(R.layout.fragm
override val contentViews by lazy { listOf(binding.constraintInside) }

override fun onContentModeInit() {
binding.vm = viewModel
binding.lifecycleOwner = this@MyPageFragment.viewLifecycleOwner
viewModel.getUserInfo()
viewModel.intent(MyPageIntent.LoadUserInfo)
addListener()
addObserver()
setResultEditNameLauncher()
Expand All @@ -50,8 +51,9 @@ class MyPageFragment : BaseVisitorFragment<FragmentMyPageBinding>(R.layout.fragm
resultEditNameLauncher =
registerForActivityResult(ActivityResultContracts.StartActivityForResult()) { result ->
if (result.resultCode == RESULT_OK) {
val name = result.data?.getStringExtra(EXTRA_NICK_NAME) ?: viewModel.nickName.value
viewModel.setNickName(name!!)
val name = result.data?.getStringExtra(EXTRA_NICK_NAME)
?: viewModel.currentState.nickname
viewModel.intent(MyPageIntent.UpdateNickname(name))
Comment thread
unam98 marked this conversation as resolved.
}
}
}
Expand All @@ -60,7 +62,7 @@ class MyPageFragment : BaseVisitorFragment<FragmentMyPageBinding>(R.layout.fragm
with(binding) {
ivMyPageEditFrame.setOnClickListener {
val intent = Intent(requireContext(), MyPageEditNameActivity::class.java)
intent.putExtra(EXTRA_NICK_NAME, "${viewModel.nickName.value}")
intent.putExtra(EXTRA_NICK_NAME, viewModel.currentState.nickname)
val stampResId = getStampResourceId()
intent.putExtra(EXTRA_PROFILE, stampResId)
resultEditNameLauncher.launch(intent)
Expand Down Expand Up @@ -89,29 +91,35 @@ class MyPageFragment : BaseVisitorFragment<FragmentMyPageBinding>(R.layout.fragm
}

private fun moveToSettingFragment() {
val bundle = Bundle().apply { putString(ACCOUNT_INFO_TAG, viewModel.email.value) }
val bundle = Bundle().apply { putString(ACCOUNT_INFO_TAG, viewModel.currentState.email) }
requireActivity().supportFragmentManager.commit {
this.setCustomAnimations(R.anim.slide_in_right, R.anim.slide_out_left)
replace<MySettingFragment>(R.id.fl_main, args = bundle)
}
}

private fun addObserver() {
viewModel.nickName.observe(viewLifecycleOwner) { nickName ->
binding.tvMyPageUserName.text = nickName.toString()
repeatOnStarted {
viewModel.state.collectLatest { state ->
bindState(state)
}
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

viewModel.userInfoState.observe(viewLifecycleOwner) {
when (it) {
UiState.Empty -> setLoadingState(false)
UiState.Loading -> setLoadingState(true)
UiState.Success -> {
setLoadingState(false)
val stampResId = getStampResourceId()
viewModel.setProfileImg(stampResId)
}
UiState.Failure -> setLoadingState(false)
private fun bindState(state: MyPageUiState) {
setLoadingState(state.isLoading)

if (!state.isLoading) {
with(binding) {
tvMyPageUserName.text = state.nickname
tvMyPageUserLv.text = state.level
pbMyPageProgress.progress = state.levelPercent
tvMyPageProgressCurrent.text = state.levelPercent.toString()
ivMyPageProfile.load(state.profileImgResId)
}

val stampResId = getStampResourceId()
viewModel.intent(MyPageIntent.UpdateProfileImg(stampResId))
}
}

Expand All @@ -122,7 +130,7 @@ class MyPageFragment : BaseVisitorFragment<FragmentMyPageBinding>(R.layout.fragm

private fun getStampResourceId(): Int {
return requireContext().getStampResId(
stampId = viewModel.stampId.value,
stampId = viewModel.currentState.stampId,
resNameParam = RES_NAME,
resType = RES_STAMP_TYPE,
packageName = requireContext().packageName
Expand Down Expand Up @@ -151,4 +159,4 @@ class MyPageFragment : BaseVisitorFragment<FragmentMyPageBinding>(R.layout.fragm
const val EXTRA_PROFILE = "profile_img"
const val ACCOUNT_INFO_TAG = "accountInfo"
}
}
}
Original file line number Diff line number Diff line change
@@ -1,66 +1,48 @@
package com.runnect.runnect.presentation.mypage

import androidx.lifecycle.LiveData
import androidx.lifecycle.MutableLiveData
import com.runnect.runnect.R
import com.runnect.runnect.domain.common.toLog
import com.runnect.runnect.domain.repository.UserRepository
import com.runnect.runnect.presentation.base.BaseViewModel
import com.runnect.runnect.presentation.state.UiState
import com.runnect.runnect.util.extension.collectResult
import com.runnect.runnect.presentation.base.MviViewModel
import dagger.hilt.android.lifecycle.HiltViewModel
import kotlinx.coroutines.flow.onStart
import javax.inject.Inject

@HiltViewModel
class MyPageViewModel @Inject constructor(
private val userRepository: UserRepository
) : BaseViewModel() {

val nickName: MutableLiveData<String> = MutableLiveData<String>()
val stampId: MutableLiveData<String> = MutableLiveData<String>(STAMP_LOCK)
val profileImgResId: MutableLiveData<Int> = MutableLiveData<Int>(R.drawable.user_profile_basic)
val level: MutableLiveData<String> = MutableLiveData<String>()
val levelPercent: MutableLiveData<Int> = MutableLiveData<Int>()
val email: MutableLiveData<String> = MutableLiveData<String>()

private val _userInfoState = MutableLiveData<UiState>(UiState.Loading)
val userInfoState: LiveData<UiState>
get() = _userInfoState

val errorMessage = MutableLiveData<String>()
fun setNickName(nickName: String) {
this.nickName.value = nickName
}

fun setProfileImg(profileImgResId: Int) {
this.profileImgResId.value = profileImgResId
) : MviViewModel<MyPageUiState, MyPageIntent, MyPageEffect>(MyPageUiState()) {

override suspend fun handleIntent(intent: MyPageIntent) {
when (intent) {
is MyPageIntent.LoadUserInfo -> loadUserInfo()
is MyPageIntent.UpdateNickname -> reduce { copy(nickname = intent.nickname) }
is MyPageIntent.UpdateProfileImg -> reduce { copy(profileImgResId = intent.resId) }
}
}

fun getUserInfo() = launchWithHandler {
userRepository.getUserInfo()
.onStart {
_userInfoState.value = UiState.Loading
}.collectResult(
onSuccess = { user ->
user.let {
level.value = it.level.toString()
nickName.value = it.nickname
stampId.value = it.latestStamp
levelPercent.value = it.levelPercent
email.value = it.email
}

_userInfoState.value = UiState.Success
},
onFailure = {
errorMessage.value = it.toLog()
_userInfoState.value = UiState.Failure
private fun loadUserInfo() {
collectFlow(
flow = { userRepository.getUserInfo() },
onLoading = {
reduce { copy(isLoading = true, error = null) }
},
onSuccess = { user ->
reduce {
copy(
Comment on lines +29 to +30
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 파일에서 전반적으로 reduce랑 copy가 많이 쓰이는데 역할이 뭐고 이유가 뭔지?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MVI에서 상태 변경은 반드시 reduce를 통해서만 이루어집니다.

  • reduce { copy(nickname = "새이름") }는 현재 상태를 기반으로 변경된 부분만 교체한 새 객체를 만드는 패턴
  • copy()는 Kotlin data class의 기본 기능 — 불변 상태 유지하면서 특정 필드만 변경
  • 이렇게 하면 상태 변경 경로가 단일화되어, 어디서 상태가 바뀌었는지 추적이 쉬워짐 (LiveData처럼 여러 곳에서 .value = 하는 것과 대비)
  • Kotlin data class copy 공식 문서

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reduce랑 copy는 세트인가? 그렇다면 이걸 하나로 합쳐서 편하게 쓸 수 있는 api를 하나 만들면 좋을 듯

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

현재 reduce가 이미 그 역할입니다.

// reduce 하나로 "현재 상태에서 이 필드만 바꿔" 를 표현
reduce { copy(nickname = "새이름") }
  • reduce는 베이스 클래스의 API
  • copy는 Kotlin data class의 기본 기능 (별도 구현 아님)
  • 둘을 더 합치려면 updateState(nickname = "새이름") 같은 형태가 되는데, data class 필드가 화면마다 다르므로 제네릭하게 만들 수 없음
  • 결론: 현재 reduce { copy(...) }가 가장 간결한 형태입니다

isLoading = false,
nickname = user.nickname,
stampId = user.latestStamp,
level = user.level.toString(),
levelPercent = user.levelPercent,
email = user.email,
error = null
)
}
)
}

companion object {
const val STAMP_LOCK = "lock"
},
onFailure = { throwable ->
val message = throwable.toLog()
reduce { copy(isLoading = false, error = message) }
postEffect(MyPageEffect.ShowError(message))
}
)
}
}
}
Loading
Loading