Conversation
1. Column으로 감싸서 한 블록으로 만들기 2. start와 end 패딩을 통합하고, 너무 큰 end 패딩 (97.dp, 272.dp)을 제거 -> padding(horizontal = 20.dp) 적용
하다보니까 preview 동작도 안하고해서 새로 만들기로함
근데 모듈 인식을 잘 못한다..
197~225번줄 코드를 추가하면 preview가 동작을 안합니다..!
197~225번줄 코드를 추가하면 preview가 동작을 안합니다..!
IntrinsicSize.Min을 제거하고 fillMaxWidth와 height를 사용합니다.
ChipDefaults.filterChipColors이 아니라 FilterChipDefaults를 써야함 왠만하면 material3쓰니가 둘의 차이를 명확히 구분해야함
agvber
reviewed
Mar 10, 2025
agvber
reviewed
Mar 10, 2025
| ) { | ||
| DayOfWeek.values().forEach { day -> | ||
| FilterChip( | ||
| selected = selectedDays.contains(day), |
Member
There was a problem hiding this comment.
비용이 많이 들어가는 작업 처럼 보이는데 remember 처리하는 게 좋지 않을까요??
화면이 STOPPED 상태로 가면 이벤트 수집을 자동으로 중단하여 불필요한 리소스 사용을 방지함. 다시 STARTED로 돌아오면 이벤트 수집을 재개함.
remember를 통해 인스턴스를 한번만 생성하게 해서 재생성을 방지한다. Compose에서 특정 상태가 변경될 때, 해당 UI 부분만 다시 그리는 과정을 recomposition이라고 하는데 remember를 통해 불필요한 객체 재생성을 방지한다.
기존에 selectDays를 rememberSaveable로 유지하여 프로세스 정료 후에도 선택 상태를 복원할 수 있도록 하는데, 직접 참조하던 DayOfWeek.values()를 remember로 감싸 불필요한 values() 호출을 방지함으로써 리컴포지션을 최적화한다.
### 🔍 문제 분석 및 개선점
**1️⃣ TimePickerDialog가 즉시 실행되는 문제**
* LaunchedEffect(Unit) 내부에서 timePickerDialog.show()를 호출하면서, TimePickerDialog가 remember 시점에서 바로 실행됨.
* LaunchedEffect는 Composition이 시작될 때 실행되므로, TimePickerDialog가 조건 없이 실행되는 문제가 발생할 수 있음.
**🔹 해결책**
* LaunchedEffect를 제거하고, showDialog 상태를 if (showDialog) 블록으로 감싸 TimePickerDialog가 필요할 때만 실행됨.
- 불필요한 Composition 트리거 방지.
**2️⃣ Dialog가 닫힐 때** **onDismissRequest()****가 불필요하게 호출될 가능성**
* TimePickerDialog의 setOnDismissListener에서 onDismissRequest()를 호출하도록 설정했는데, 사용자가 onConfirm을 통해 시간을 선택할 때도 setOnDismissListener가 실행될 가능성이 있음.
* 결과적으로 onDismissRequest가 중복 호출되거나 의도치 않은 동작이 발생할 위험이 있음.
⠀**🔹 해결책**
* TimePickerDialog를 표시하는 로직을 isTimePickerDialogShow가 true일 때 실행되도록 제어하고, dismiss() 호출을 명확히 관리하고, onDismissRequest()가 setOnDismissListener 내부에서만 실행되도록 함
**3️⃣ 시간이 두 자리 수가 아닐 때 포맷 문제**
* 현재 time?.let { "${it.hour}:${it.minute}" }에서 hour와 minute 값이 한 자리 수일 경우 9:5와 같이 표시될 수 있음.
* 일반적인 UX 기대치는 09:05와 같이 두 자리 형식을 유지하는 것.
⠀**🔹 해결책**
* String.format("%02d:%02d", hour, minute)을 사용해 2자리 형식을 유지. 9:5가 아니라 09:05처럼 표시됨.
### 1.remember 최소화의 의미
remember를 최소화한다는 것은 **불필요한 상태 유지(state preservation)를 줄이는 것**을 의미해. remember는 Composable 함수가 재구성될 때 값을 유지하도록 도와주지만, 모든 상태를 remember로 관리할 필요는 없어.
✅ **이 코드에서 불필요한** **remember** **예시**
```kotlin
val calendar = remember { Calendar.getInstance() }
val hour = remember { calendar.get(Calendar.HOUR_OF_DAY) }
val minute = remember { calendar.get(Calendar.MINUTE) }
```
위 코드에서 calendar를 remember로 감싸는 것은 큰 의미가 없어. 왜냐하면 Calendar.getInstance()는 단순한 객체 생성을 수행할 뿐이고, 해당 값을 유지할 필요가 없기 때문이야. 대신 아래처럼 변경하는 게 좋겠지:
🔹 **개선된 코드**
```kotlin
val calendar = Calendar.getInstance()
val hour = calendar.get(Calendar.HOUR_OF_DAY)
val minute = calendar.get(Calendar.MINUTE)
```
이렇게 하면 calendar 자체를 remember할 필요 없이, 매번 새로운 값을 가져오게 돼.
**🚨 문제점:**
* TimePickerDialog는 **Android의** Dialog **객체이므로 리소스를 차지**하는데, 이 객체가 **재구성(recomposition)** 되어도 계속 살아남을 수 있어.
* TimePickerDialog가 **화면에서 사라질 때 명확하게 dismiss()를 호출하지 않으면** 메모리 누수(memory leak)나 예기치 않은 동작이 발생할 수 있어.
⠀✅ **해결책:** DisposableEffect **사용**
DisposableEffect를 사용하면 Composable이 **제거될 때(**onDispose**)** dismiss()**를 호출**할 수 있어.
🔹 **이렇게 개선하면?**
* show()를 별도로 호출하지 않아도 DisposableEffect 내부에서 자동으로 실행됨.
* Compose가 관리하는 리소스가 아니므로, **명확하게 해제하는 코드가 필요함** → onDispose 사용.
* DisposableEffect는 LaunchedEffect처럼 한 번 실행되지만, **recomposition이 발생해도 중복 실행되지 않음.**
→ 다이얼로그가 불필요하게 여러 개 생기지 않음.
### ✅ Recomposition이 되면 사라지는 것 vs. 사라지지 않는 것
맞아! Jetpack Compose에서 recomposition이 발생하면 **일부 값들은 새로 생성되고 사라지지만**, Android의 일부 객체들은 **계속 남아있을 수 있어.**
| **항목** | **Recomposition 시 유지됨?** | **설명** |
|:-:|:-:|:-:|
| val number = 10 | ❌ 사라짐 | 그냥 지역 변수이므로 recomposition 때마다 다시 초기화됨 |
| remember { mutableStateOf(0) } | ✅ 유지됨 | remember를 사용하면 상태를 유지할 수 있음 |
| DisposableEffect { MediaPlayer() } | ✅ 유지됨 (onDispose에서 해제해야 함) | MediaPlayer() 같은 리소스 객체는 **recomposition으로 사라지지 않음**, 직접 해제 필요 |
| TimePickerDialog(context, ...) | ✅ 유지됨 (onDispose에서 dismiss 필요) | Dialog는 Compose가 관리하는 게 아니므로 명시적으로 dismiss() 해줘야 함 |
### 📌 현재 코드 문제점
너의 기존 코드에서는 TimePickerDialog를 일반적인 Composable처럼 if (isTimePickerDialogShow) 안에서 직접 호출했어.
하지만 **다이얼로그는 상태 변화에 의해 재구성되면 안 돼!**
왜냐하면, Composable 함수는 Recomposition(재구성)이 일어날 수 있는데,
그럴 때마다 새로운 다이얼로그 객체가 계속 생성되면서 문제가 발생할 가능성이 높아.
# 🚀 1. Composable에서 제거한다는 의미
기존 코드처럼 TimePickerDialog를 Composable 안에서 호출하면 **재구성될 때마다 계속 생성**됨.
이를 방지하기 위해 Composable 함수에서 TimePickerDialog를 **없애고**, 일반 함수(function)로 만든다는 의미야.
### ✍️ 예제 (잘못된 코드)
```kotlin
if (isTimePickerDialogShow) {
TimePickerDialog(
onDismissRequest = { isTimePickerDialogShow = false },
onConfirm = { hour, minute ->
time = LocalTime.of(hour, minute)
isTimePickerDialogShow = false
}
)
}
```
이렇게 Composable 안에서 다이얼로그를 직접 호출하면, isTimePickerDialogShow가 변경될 때마다 **계속 재구성됨**.
그래서 다이얼로그는 따로 일반 함수에서 관리해야 해!
# 🚀 2. DisposableEffect 기반으로 변경한다는 의미
### DisposableEffect는 **Side Effect(부수 효과)를 안전하게 관리**하는 Composable API야.
**다이얼로그는 UI 요소가 아니라 부수 효과(Side Effect)이므로,** DisposableEffect**에서 다룬다**는 의미야.
## 🎯 결론
* ✅ **TimePickerDialogEffect****를 제거하고 일반 함수(****showTimePickerDialog()****)로 변경**
* ✅ **DisposableEffect** **안에서 다이얼로그를 관리 → UI 재구성과 무관하게 동작**
* ✅ **이제** **isTimePickerDialogShow****가** **true****일 때 한 번만 다이얼로그 실행됨**
현재 코드 흐름 1. DailyEditRoute에서 viewModel.dailyEditUiState를 uiState로 사용 2. DailyEditScreen에서 selectedDays를 uiState.selectedDays로 전달 3. onDaySelected로 ViewModel의 toggleSelectedDay(day)를 호출하여 상태 업데이트
agvber
reviewed
Apr 4, 2025
| } | ||
|
|
||
| @Composable | ||
| fun TimePickerDialogEffect( |
Member
There was a problem hiding this comment.
DisposableEffect 말고 그냥 아래와 같은 방식으로 처리하면 안되나요?
왜냐하면 isTimePickerDialogShow == true일때마다 새로운 인스턴스를 계속 참조하는게 좋지는 않아보입니다.
@Composable
internal fun DailyEditScreen(
) {
val timePickerDialog = remember {
TimePickerDialog(
context,
{ _, selectedHour, selectedMinute -> onConfirm(selectedHour, selectedMinute) },
hour,
minute,
true
)
}
Row(
verticalAlignment = Alignment.CenterVertically,
modifier = Modifier
.fillMaxWidth()
.height(52.dp)
.background(color = WH, shape = RoundedCornerShape(10.dp))
.border(width = 1.dp, color = G2, shape = RoundedCornerShape(10.dp))
.padding(horizontal = 20.dp)
.noRippleClickable { timePickerDialog.show() }
) {
agvber
reviewed
Apr 4, 2025
| ) | ||
| } | ||
|
|
||
| LaunchedEffect(viewModel.event) { |
Member
There was a problem hiding this comment.
이거 viewModel.event 값이 변경될때마다 LauchedEffect가 재실행 될 가능성이 높아보여요
어짜피 flow collect여서 Unit으로 처리해도 될 것 같습니다
agvber
reviewed
Apr 4, 2025
|
|
||
| @Composable | ||
| fun DailyItem( | ||
| dailyItem: DailyManage?, |
Member
There was a problem hiding this comment.
어짜피 Null checking을 앞에서 하는데 굳이 Null을 허용할 필요가 있을까요?
if (daily == null) {
CircularProgressIndicator() // 로딩 상태 처리
} else {
DailyItem(
dailyItem = daily,
onEditDailyRequest = { onEditDailyRequest(daily) },
onDeleteDailyRequest = { onDeleteDailyRequest(daily) }
)
}
Member
|
수고하셨습니다! |
remember를 활용해 TimePickerDialog 인스턴스를 재사용하면 DisposableEffect로 매번 새로 만드는 방식보다 메모리 측면에서도 깔끔하고 효율적입니다. 다만 remember는 Composable 내에서만 작동하고, context, hour, minute 등 초기화 시점에 필요한 값들이 유효한 시점에 있어야 해서 그 점도 고려해야 해요.TimePickerDialog를 remember로 컴포저블 안에서 관리하므로 DisposableEffect가 필요 없습니다.
1. viewModel.event가 바뀔 때마다 LaunchedEffect 블록이 재실행돼.
2. 굳이 viewModel.event를 key로 줄 필요 없고, 그냥 한 번만 실행되게 하면 됩니다~
3. Flow는 코틀린에서 비동기 데이터 스트림을 처리할 수 있는 도구야.
즉, 시간이 지남에 따라 데이터가 하나씩 emit(방출)될 수 있고, 우리는 그걸 collect() 해서 처리해.
viewModel.event.collect { event ->
// 새로운 이벤트가 emit될 때마다 여기가 실행됨
}
→ 이건 "viewModel에서 이벤트가 나올 때마다 처리하자!"는 뜻이야.
4. Unit은 코틀린에서 "아무 값도 없어요!"를 나타내는 특별한 타입이야.
Unit을 LaunchedEffect의 key로 쓰면, 컴포저블이 처음 실행될 때만 딱 한 번 실행돼.
#요약
viewModel.event : 변경될 수 있음 → 변경될 때마다 LaunchedEffect 재실행
Unit : 고정된 값 → LaunchedEffect는 한 번만 실행됨
collect : Flow에서 새 값이 emit될 때마다 자동 실행됨
5. 이렇게 하면 collect 안에서 알아서 이벤트를 처리하니까, 굳이 viewModel.event가 바뀔 때마다 전체 블록을 다시 실행할 필요가 없다는 뜻이야.
#LaunchedEffect
💡 LaunchedEffect는 언제 실행될까?
LaunchedEffect(key)는 이렇게 작동해:
컴포저블이 Composition에 처음 들어올 때 실행됨.
key가 변경될 때마다 다시 실행됨.
key가 같으면, Recomposition이 일어나도 재실행되지 않음.
🔁 예시로 살펴보자
kotlin
복사
편집
@composable
fun MyScreen(userId: String) {
LaunchedEffect(userId) {
println("Fetching user info for $userId")
}
// 나머지 UI...
}
처음 MyScreen("123")이 그려질 때 → LaunchedEffect("123") 실행됨
userId가 "456"으로 바뀜 → LaunchedEffect("456") 실행됨 (다시!)
그냥 recomposition 발생 (예: 텍스트 입력 등, userId는 그대로) → ❌ 실행 안 됨
🔒 그래서 Unit을 key로 쓰면?
kotlin
복사
편집
LaunchedEffect(Unit) {
// 처음 한 번만 실행
}
→ 이건 컴포저블이 처음 Composition에 들어올 때만 실행되고, 그 이후엔 절대 실행되지 않아
(심지어 recomposition이 수백 번 일어나도 다시 실행 안 됨)
✅ 이상적인 상황은?
상황 이상적인 key
단 한 번만 실행하고 싶다 (초기화, 초기 로딩 등) Unit
특정 값이 바뀔 때마다 다시 실행하고 싶다 LaunchedEffect(key) 형태
Flow를 수집할 거다 (collect) LaunchedEffect(Unit)로 한 번만 실행하고 collect {} 사용
💬 요약하면
LaunchedEffect는 "key 값"의 변화 기준으로 실행되며, Unit을 쓰면 처음 한 번만 실행되는 구조라 recomposition에도 영향을 받지 않아.
recomposition이 일어나더라도 key가 안 바뀌면 다시 실행되지 않음.
theBettor
force-pushed
the
refactor/daily
branch
from
c407691 to
d0738cd
Compare
…, local state (예: rememberSaveable)는 피하자."
🔍 피드백 내용 요약
uiState.dailyExpand.content는 이미 상태이기 때문에
→ var daily by rememberSaveable { mutableStateOf(uiState.dailyExpand.content) }
❌ 이렇게 또 상태로 감싸지 말라는 의미야.
rememberSaveable은 생명주기 복원 시 ViewModel보다 우선순위가 높기 때문에
→ Activity가 재생성되면 ViewModel의 값은 새로 초기화되지만,
→ rememberSaveable은 복원된 값을 유지해서,
→ uiState.dailyExpand.content와 daily가 서로 다른 상태가 될 수 있다.
💡 요지는 이거야:
"이미 상태인 값을 다시 상태로 만들면 상태 불일치가 생길 수 있으니, ViewModel의 상태만 사용하고, local state (예: rememberSaveable)는 피하자."
✅ 결론: 너는 이렇게 수정하면 돼
rememberSaveable과 mutableStateOf를 daily에 사용하지 말고
uiState.dailyExpand.content만 읽고,
값 변경은 ViewModel 함수로 처리해.
3. val uiState by viewModel.dailyEditUiState에서 에러 발생 시 해결
이 부분은 State<DailyEditUiState>를 by로 위임하려면 dailyEditUiState가 State<DailyEditUiState> 타입이어야 해.
확인사항:
viewModel.dailyEditUiState가 val dailyEditUiState: State<DailyEditUiState> 혹은 val dailyEditUiState = mutableStateOf(...) 여야 by가 가능함.
예시로 ViewModel 코드:
kotlin
복사
편집
class DailyEditViewModel @Inject constructor(...) : ViewModel() {
private val _dailyEditUiState = mutableStateOf(DailyEditUiState())
val dailyEditUiState: State<DailyEditUiState> = _dailyEditUiState
fun changeDailyContent(content: String) {
_dailyEditUiState.value = _dailyEditUiState.value.copy(
dailyExpand = _dailyEditUiState.value.dailyExpand.copy(content = content)
)
}
// 이벤트, selectedDays 등 추가로 구현했겠지!
}
agvber
approved these changes
Apr 22, 2025
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
3 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 내용(Issue 꼭 달기!)
resolved: #98 #106 #110 #111