Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion feature/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ android {
}

dependencies {

implementation(project(":domain"))
implementation(AndroidX.CORE)
implementation(AndroidX.LIFECYCLE_RUNTIME_KTX)
implementation(AndroidX.ACTIVITY_COMPOSE)
Expand Down
62 changes: 62 additions & 0 deletions feature/src/main/java/com/matzip/feature/base/BaseViewModel.kt
Copy link
Collaborator

Choose a reason for hiding this comment

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

BaseViewModel 전반적으로 잘 구조화해주신것 같습니다. 상태 관리가 Flow와 LiveData가 혼용되어 있는데 , 전체를 다 Flow 기반으로 통일해도 괜찮나요? 보니까 uiState와 eventFlow는 StateFlow/Flow 기반으로 되어 있는데 commonError는 LiveData로 되어 있는거 같아서요!! coderabbit에서 말한 네이밍 수정과 TODO 부분 구현만 해주시면 깔끔할 것 같습니다!! 수고하셨습니다!!

Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package com.matzip.feature.base

import androidx.lifecycle.LiveData
import androidx.lifecycle.MutableLiveData
import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import com.matzip.domain.base.ApiState
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.update
import kotlinx.coroutines.launch

abstract class BaseViewModel<STATE: PageState, EVENT>(
initialPageState : STATE,
) : ViewModel() {

private val _uiState = MutableStateFlow(initialPageState)
val uiState = _uiState.asStateFlow()

private val _eventFlow = EventFlow<EVENT>()
val eventFlow = _eventFlow.events

private val _commonError = MutableLiveData<String>()
val commonError: LiveData<String> = _commonError

protected fun updateState(state: STATE) {
viewModelScope.launch {
_uiState.update { state }
}
}
protected fun emitEventFlow(event: EVENT) {
viewModelScope.launch {
_eventFlow.emit(event)
}
}

protected fun<D> resultResponse(
response: ApiState<D>,
successCallback : (D) -> Unit,
errorCallback : ((String) -> Unit)? = null,
needLoading : Boolean = true)
{
when(response){
is ApiState.Error -> {
//TODO 에러 코드를 다룰 예정입니다.
// if(response.errorCode == StatusCode.ERROR_404 ||
// response.errorCode == StatusCode.ERROR ||
// response.errorCode == StatusCode.NETWORK_ERROR) {
// showCommonError(response.errorCode)
// }
// else errorCallback?.invoke(response.errorCode)
// endLoading()
}
// TODO Loading 상태일때 화면을 처리할 예정입니다.
ApiState.Loading -> if(needLoading) {}
is ApiState.Success -> {
successCallback.invoke(response.data)
//endLoading()
}
}
}
Comment on lines +37 to +61
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

❓ Verification inconclusive

API 응답 처리 함수의 구현을 완료해주세요.

다음 사항들을 개선해야 합니다:

  1. 빈 if 블록 제거: 55번째 줄의 빈 if 블록은 정적 분석 도구에서 지적한 대로 제거하거나 구현해야 합니다.
  2. TODO 주석 해결: 에러 코드 처리와 로딩 상태 처리 로직이 미완성 상태입니다.
  3. 주석 처리된 코드: 계획된 기능이지만 구현되지 않은 코드가 주석으로 남아있습니다.
  4. 함수명 개선: resultResponse보다 handleApiResponse가 더 명확합니다.

다음 개선 사항을 적용하시겠습니까?

-    protected fun<D> resultResponse(
+    protected fun<D> handleApiResponse(
         response: ApiState<D>,
         successCallback : (D) -> Unit,
         errorCallback : ((String) -> Unit)? = null,
         needLoading : Boolean = true)
     {
         when(response){
             is ApiState.Error -> {
-                //TODO 에러 코드를 다룰 예정입니다.
+                // TODO: 에러 코드 처리 로직 구현 필요
+                errorCallback?.invoke("API 에러 발생") // 임시 구현
             }
-            // TODO Loading 상태일때 화면을 처리할 예정입니다.
-            ApiState.Loading -> if(needLoading) {}
+            ApiState.Loading -> {
+                if(needLoading) {
+                    // TODO: 로딩 상태 처리 로직 구현 필요
+                }
+            }
             is ApiState.Success -> {
                 successCallback.invoke(response.data)
-                //endLoading()
+                // TODO: 로딩 종료 처리 필요
             }
         }
     }

에러 처리 및 로딩 상태 관리 로직을 구현하는 데 도움이 필요하시면 알려주세요.


API 응답 처리 함수 구현 완료 및 리팩토링 필요
파일: feature/src/main/java/com/matzip/feature/base/BaseViewModel.kt (37–61줄)
다음 사항을 반영해주세요:

  • 빈 if 블록 제거 혹은 로직 추가 (Loading 상태)
  • TODO 주석 제거 및 실제 에러/로딩 처리 로직 구현
  • 주석 처리된 코드 정리 (에러 코드별 처리 분기)
  • 함수명 resultResponsehandleApiResponse로 변경 검토
-    protected fun<D> resultResponse(
+    protected fun<D> handleApiResponse(
         response: ApiState<D>,
         successCallback: (D) -> Unit,
         errorCallback: ((String) -> Unit)? = null,
         needLoading: Boolean = true
     ) {
         when (response) {
             is ApiState.Error -> {
-                //TODO 에러 코드를 다룰 예정입니다.
+                // TODO: API 에러 처리 로직 구현 필요
+                errorCallback?.invoke(response.errorCode)
+                // endLoading()
             }
-            // TODO Loading 상태일때 화면을 처리할 예정입니다.
-            ApiState.Loading -> if (needLoading) {}
+            ApiState.Loading -> {
+                if (needLoading) {
+                    // TODO: 로딩 표시 로직 구현 필요
+                }
+            }
             is ApiState.Success -> {
-                successCallback.invoke(response.data)
-                //endLoading()
+                successCallback.invoke(response.data)
+                // TODO: 로딩 종료 처리 구현 필요
             }
         }
     }

필요하시면 에러/로딩 처리 구현 부분에 대해 추가 지원을 드릴 수 있습니다.

📝 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.

Suggested change
protected fun<D> resultResponse(
response: ApiState<D>,
successCallback : (D) -> Unit,
errorCallback : ((String) -> Unit)? = null,
needLoading : Boolean = true)
{
when(response){
is ApiState.Error -> {
//TODO 에러 코드를 다룰 예정입니다.
// if(response.errorCode == StatusCode.ERROR_404 ||
// response.errorCode == StatusCode.ERROR ||
// response.errorCode == StatusCode.NETWORK_ERROR) {
// showCommonError(response.errorCode)
// }
// else errorCallback?.invoke(response.errorCode)
// endLoading()
}
// TODO Loading 상태일때 화면을 처리할 예정입니다.
ApiState.Loading -> if(needLoading) {}
is ApiState.Success -> {
successCallback.invoke(response.data)
//endLoading()
}
}
}
protected fun<D> handleApiResponse(
response: ApiState<D>,
successCallback: (D) -> Unit,
errorCallback: ((String) -> Unit)? = null,
needLoading: Boolean = true
) {
when (response) {
is ApiState.Error -> {
// TODO: API 에러 처리 로직 구현 필요
errorCallback?.invoke(response.errorCode)
// endLoading()
}
ApiState.Loading -> {
if (needLoading) {
// TODO: 로딩 표시 로직 구현 필요
}
}
is ApiState.Success -> {
successCallback.invoke(response.data)
// TODO: 로딩 종료 처리 구현 필요
}
}
}
🧰 Tools
🪛 detekt (1.23.8)

[warning] 55-55: This empty block of code can be removed.

(detekt.empty-blocks.EmptyIfBlock)

🤖 Prompt for AI Agents
In feature/src/main/java/com/matzip/feature/base/BaseViewModel.kt between lines
37 and 61, refactor the resultResponse function by renaming it to
handleApiResponse for clarity, remove all TODO comments, and implement the error
and loading handling logic instead of empty or commented blocks. Specifically,
replace the empty if block for ApiState.Loading with actual loading state
handling, restore and clean up the error code handling logic for ApiState.Error,
and remove commented-out code while ensuring the function properly calls
successCallback on success and handles endLoading or similar UI state updates as
needed.

}
53 changes: 53 additions & 0 deletions feature/src/main/java/com/matzip/feature/base/Event.kt
Copy link
Collaborator

Choose a reason for hiding this comment

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

Event 클래스의 역할이 명확하게 잘 구현되어 있어서, 단일 소비 이벤트를 깔끔하게 처리할 수 있는 것 같습니다. codeRabbit에서 말한 멀티스레드 환경에서 동시 접근할 경우 race conditon에 노출될 수 있는것 같아서, 동기화 처리를 해보는 것도 좋을 것 같습니다 전반적으로 좋은 코드 감사합니다!!

Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package com.matzip.feature.base

import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.SharedFlow
import kotlinx.coroutines.flow.asSharedFlow

// 이벤트 단일 소비를 위한 래퍼 클래스
class Event<out T>(private val content: T) {

private var hasBeenHandled = false

fun getContentIfNotHandled(): T? {
return if (hasBeenHandled) {
null
} else {
hasBeenHandled = true
content
}
}

fun peekContent(): T = content
}
Comment on lines +9 to +23
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Event 클래스의 스레드 안전성을 개선해주세요.

hasBeenHandled 플래그가 스레드 안전하지 않습니다. 멀티스레드 환경에서 동일한 이벤트가 여러 번 처리될 수 있습니다.

+import java.util.concurrent.atomic.AtomicBoolean
+
 class Event<out T>(private val content: T) {
 
-    private var hasBeenHandled = false
+    private val hasBeenHandled = AtomicBoolean(false)
 
     fun getContentIfNotHandled(): T? {
-        return if (hasBeenHandled) {
+        return if (hasBeenHandled.getAndSet(true)) {
             null
         } else {
-            hasBeenHandled = true
             content
         }
     }
📝 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.

Suggested change
class Event<out T>(private val content: T) {
private var hasBeenHandled = false
fun getContentIfNotHandled(): T? {
return if (hasBeenHandled) {
null
} else {
hasBeenHandled = true
content
}
}
fun peekContent(): T = content
}
import java.util.concurrent.atomic.AtomicBoolean
class Event<out T>(private val content: T) {
private val hasBeenHandled = AtomicBoolean(false)
fun getContentIfNotHandled(): T? {
return if (hasBeenHandled.getAndSet(true)) {
null
} else {
content
}
}
fun peekContent(): T = content
}
🤖 Prompt for AI Agents
In feature/src/main/java/com/matzip/feature/base/Event.kt around lines 9 to 23,
the hasBeenHandled flag is not thread-safe, which can cause the same event to be
handled multiple times in a multithreaded environment. To fix this, make the
hasBeenHandled flag access atomic by using synchronization or atomic variables
such as AtomicBoolean. Update the getContentIfNotHandled() method to check and
set the flag atomically to ensure only one thread can handle the event.


// 이벤트 처리를 위한 Flow 클래스
class EventFlow<T> {
private val _events = MutableSharedFlow<Event<T>>(
// 이벤트를 단일 소비로 처리하기 위해
replay = 1,
// 버퍼 크기를 1로 설정하여 최신 이벤트만 유지
extraBufferCapacity = 1
)

val events: SharedFlow<Event<T>> = _events.asSharedFlow()

suspend fun emit(value: T) {
_events.emit(Event(value))
}

fun tryEmit(value: T) {
_events.tryEmit(Event(value))
}
}
Comment on lines +26 to +43
Copy link

@coderabbitai coderabbitai bot Jul 11, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

EventFlow 설정에 잠재적 문제가 있습니다.

replay = 1extraBufferCapacity = 1의 조합이 예상과 다르게 동작할 수 있습니다. 이 설정은 구독자가 없어도 이벤트가 버퍼에 유지되어 나중에 구독하는 경우 이전 이벤트를 받을 수 있습니다.

단일 소비 이벤트가 목적이라면 다음과 같이 수정을 고려해보세요:

 private val _events = MutableSharedFlow<Event<T>>(
-    // 이벤트를 단일 소비로 처리하기 위해
-    replay = 1,
-    // 버퍼 크기를 1로 설정하여 최신 이벤트만 유지
-    extraBufferCapacity = 1
+    // 단일 소비를 위해 replay 없이 설정
+    replay = 0,
+    extraBufferCapacity = 1
 )
📝 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.

Suggested change
class EventFlow<T> {
private val _events = MutableSharedFlow<Event<T>>(
// 이벤트를 단일 소비로 처리하기 위해
replay = 1,
// 버퍼 크기를 1로 설정하여 최신 이벤트만 유지
extraBufferCapacity = 1
)
val events: SharedFlow<Event<T>> = _events.asSharedFlow()
suspend fun emit(value: T) {
_events.emit(Event(value))
}
fun tryEmit(value: T) {
_events.tryEmit(Event(value))
}
}
class EventFlow<T> {
private val _events = MutableSharedFlow<Event<T>>(
// 단일 소비를 위해 replay 없이 설정
replay = 0,
extraBufferCapacity = 1
)
val events: SharedFlow<Event<T>> = _events.asSharedFlow()
suspend fun emit(value: T) {
_events.emit(Event(value))
}
fun tryEmit(value: T) {
_events.tryEmit(Event(value))
}
}
🤖 Prompt for AI Agents
In feature/src/main/java/com/matzip/feature/base/Event.kt around lines 26 to 43,
the combination of replay = 1 and extraBufferCapacity = 1 in MutableSharedFlow
causes events to be buffered and replayed to new subscribers, which conflicts
with the intended single consumption behavior. To fix this, set replay to 0 to
prevent replaying past events and adjust extraBufferCapacity as needed to allow
buffering without replay, ensuring events are consumed only once by active
subscribers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

그러면 딜레이 이슈로 구독 전 이벤트 발생했을 때 누락되는 문제가 발생하는거 아님?

Copy link

Choose a reason for hiding this comment

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

@rudtjr1106 맞습니다! 정확한 지적이에요.

replay = 0으로 설정하면 구독 전에 발생한 이벤트는 누락되는 문제가 있습니다. 이는 다음과 같은 상황에서 발생할 수 있어요:

  • 화면 초기화 지연
  • 코루틴 스코프 설정 지연
  • 컴포저블 생성 타이밍 이슈

현재 구현의 trade-off:

  • replay = 1: 단일 소비 원칙은 위배되지만 이벤트 누락 방지
  • replay = 0: 단일 소비는 보장되지만 타이밍 이슈로 이벤트 누락 가능

대안 방안들:

  1. 현재 구현 유지 - 이벤트 누락 방지를 우선시
  2. Channel 사용 - 자연스러운 큐잉으로 타이밍 이슈 해결
  3. 용도별 분리 - 중요한 이벤트는 replay, 일반 이벤트는 replay 0

실제 사용 패턴에서 어떤 것이 더 중요한지에 따라 결정하는 것이 좋겠네요. 현재 구현이 더 안전한 선택일 수 있습니다.


✏️ Learnings added
Learnt from: rudtjr1106
PR: the-spot-campus/MatzipBook-Android#10
File: feature/src/main/java/com/matzip/feature/base/Event.kt:26-43
Timestamp: 2025-07-11T02:28:30.612Z
Learning: EventFlow에서 replay = 1 vs replay = 0 설정은 트레이드오프가 있음. replay = 1은 단일 소비 원칙을 위배하지만 구독 전 이벤트 누락을 방지함. replay = 0은 단일 소비를 보장하지만 타이밍 이슈로 이벤트가 누락될 수 있음.

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.


suspend inline fun <T> Flow<Event<T>>.collectEvent(
crossinline onEvent: (T) -> Unit
) {
collect { event ->
event.getContentIfNotHandled()?.let { value ->
onEvent(value)
}
}
}
5 changes: 5 additions & 0 deletions feature/src/main/java/com/matzip/feature/base/PageState.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package com.matzip.feature.base

interface PageState{
object Default: PageState
}
5 changes: 0 additions & 5 deletions feature/src/main/java/com/matzip/feature/base/base.kt

This file was deleted.

16 changes: 16 additions & 0 deletions feature/src/main/java/com/matzip/feature/designSystem/Color.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package com.matzip.feature.designSystem

import androidx.compose.ui.graphics.Color

val Background = Color(0xFFFAFAFA)
val Box = Color(0xFFF0F0F0)
val Main = Color(0xFFFF8400)
val MainLight1 = Color(0xFFFFB273)
val MainLight2 = Color(0xFFFFDABB)
val Black = Color(0xFF111111)
val BlackBox = Color(0xFF282828)
val DarkGray = Color(0xFF3C3C3C)
val TextSub1 = Color(0xFF787878)
val TextSub2 = Color(0xFFA8A8A8)
val LightGray = Color(0xFFD9D9D9)
val Sub1 = Color(0xFFFFD000)
110 changes: 110 additions & 0 deletions feature/src/main/java/com/matzip/feature/designSystem/Typo.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
package com.matzip.feature.designSystem

import androidx.compose.ui.text.TextStyle
import androidx.compose.ui.text.font.Font
import androidx.compose.ui.text.font.FontFamily
import androidx.compose.ui.text.font.FontWeight
import androidx.compose.ui.unit.sp
import com.matzip.feature.R

val matZipFontFamily = FontFamily(
Font(R.font.pretendard_bold, FontWeight.Bold),
Font(R.font.pretendard_light, FontWeight.Light),
Font(R.font.pretendard_medium, FontWeight.Medium),
Font(R.font.pretendard_semi_bold, FontWeight.SemiBold),
Font(R.font.pretendard_regular, FontWeight.Normal),
)

object MatZipTypography {
val h1: TextStyle = TextStyle(
fontFamily = matZipFontFamily,
fontWeight = FontWeight.Bold,
lineHeight = 43.2.sp,
fontSize = 32.sp
)

val h2: TextStyle = TextStyle(
fontFamily = matZipFontFamily,
fontWeight = FontWeight.Bold,
lineHeight = 37.8.sp,
fontSize = 28.sp
)

val title1: TextStyle = TextStyle(
fontFamily = matZipFontFamily,
fontWeight = FontWeight.Bold,
lineHeight = 32.4.sp,
fontSize = 24.sp
)

val title2: TextStyle = TextStyle(
fontFamily = matZipFontFamily,
fontWeight = FontWeight.Bold,
lineHeight = 27.sp,
fontSize = 20.sp
)

val title3: TextStyle = TextStyle(
fontFamily = matZipFontFamily,
fontWeight = FontWeight.Bold,
lineHeight = 21.6.sp,
fontSize = 16.sp
)

val title4: TextStyle = TextStyle(
fontFamily = matZipFontFamily,
fontWeight = FontWeight.Bold,
lineHeight = 18.2.sp,
fontSize = 14.sp
)

val title5: TextStyle = TextStyle(
fontFamily = matZipFontFamily,
fontWeight = FontWeight.SemiBold,
lineHeight = 18.2.sp,
fontSize = 14.sp
)

val title6: TextStyle = TextStyle(
fontFamily = matZipFontFamily,
fontWeight = FontWeight.Bold,
lineHeight = 15.6.sp,
fontSize = 12.sp
)

val title7: TextStyle = TextStyle(
fontFamily = matZipFontFamily,
fontWeight = FontWeight.SemiBold,
lineHeight = 15.6.sp,
fontSize = 12.sp
)

val body1: TextStyle = TextStyle(
fontFamily = matZipFontFamily,
fontWeight = FontWeight.Normal,
lineHeight = 21.6.sp,
fontSize = 16.sp
)

val body2: TextStyle = TextStyle(
fontFamily = matZipFontFamily,
fontWeight = FontWeight.Normal,
lineHeight = 18.9.sp,
fontSize = 14.sp
)

val ex1: TextStyle = TextStyle(
fontFamily = matZipFontFamily,
fontWeight = FontWeight.Normal,
lineHeight = 15.6.sp,
fontSize = 12.sp
)

val ex2: TextStyle = TextStyle(
fontFamily = matZipFontFamily,
fontWeight = FontWeight.Normal,
lineHeight = 13.sp,
fontSize = 10.sp
)

}

This file was deleted.

Binary file added feature/src/main/res/font/pretendard_bold.ttf
Binary file not shown.
Binary file added feature/src/main/res/font/pretendard_light.ttf
Binary file not shown.
Binary file added feature/src/main/res/font/pretendard_medium.ttf
Binary file not shown.
Binary file added feature/src/main/res/font/pretendard_regular.ttf
Binary file not shown.
Binary file not shown.