[세션] 복수의 발표자 대응 #302
[세션] 복수의 발표자 대응 #302
Conversation
Test Results19 tests 19 ✅ 5s ⏱️ Results for commit 7dc29d8. ♻️ This comment has been updated with latest results. |
| @@ -49,8 +49,7 @@ internal fun WidgetSessionCard(session: Session) { | |||
| ) | |||
| Spacer(modifier = GlanceModifier.width(4.dp)) | |||
| Text( | |||
| // FIXME : 2명 이상 발표자 있는 case에 대해 정상 동작하도록 수정 필요 | |||
| session.speakers.first().name, | |||
| session.speakers.joinToString { it.name }, | |||
There was a problem hiding this comment.
리컴포지션이 일어날때마다 joinToString을 다시 진행합니다.
최적화를 해주세요.
There was a problem hiding this comment.
@taehwandev
위젯에서는 도메인 모델인 Session을 그대로 사용하는 것으로 보입니다.
도메인 모델인 Session에 val sessionString get() = session.speakers.joinToString { it.name }를 두면 여러 UiState에서 굳이 가공하지 않아도 될 것 같은데 이 도메인 모델을 수정해도 괜찮을까요?
아니면 WidgetUiState를 새로 생성하는 것을 선호하시나요?
There was a problem hiding this comment.
이 부분은 선호도가 아닌
도메인 - UI과는 무관한 데이터 제공
ViewModel에서 UI를 처리하니 UiState를 활용한 데이터 처리가 적합한 부분입니다.
UI의 데이터가 다르게 사용될건데 이게 도메인이 개입할 이유는 없습니다.
There was a problem hiding this comment.
@taehwandev
해당 부분은 ViewModel이 없으며, Composable 내부에서 State에 UseCase로 직접 도메인 모델을 받고있습니다.
그러므로 성능 최적화에는 세가지 선택지가 있다고 생각합니다.
- ViewModel와 UiState를 생성해, 거기서 데이터 처리를 관리.
- 위젯이라 VM 생성이 불가능?
- 어차피 여러 곳에서 사용하니 Domain Model을 보다 UI에 적합하도록 변경.
- Composable 내부에서 derivedStateOf를 사용해 가공.
위의 세가지 선택지는 기존 프로젝트 방침에 따라 결정된다고 생각하는데 어떻게 생각하시나요?
제 인식이 틀리면 죄송합니다.
There was a problem hiding this comment.
적절한 방버은 아래 두가지일것 같네요.
- ViewModel와 UiState를 생성해, 거기서 데이터 처리를 관리.
- 위젯이라 VM 생성이 불가능?
- Composable 내부에서 derivedStateOf를 사용해 가공.
Domain Model을 보다 UI에 적합하도록... 지금 구조에서의 적합한 방법은 아닐 수도 있어 보이지 만 동일한 데이터 형태로 출력 결과만 달라지는 부분이라 모두를 수정한다기보단 widget 용을 새로 파는것도 고려는 가능할것 같습니다.
그리고 컴포즈에선 컴포즈 최적화를 위한 어노테이션을 제공합니다. 그걸 활용하기 위해선 UiState 관리를 하면서 어노테이션을 관리할 수 있구요. 그럼 결국 모델과는 별도로 분리가 일어날 수 있습니다.
There was a problem hiding this comment.
가장 쉬운 방법으로 update 일어났을때의 대응 부분으로 최적화가 가능합니다.
domain 까지 가서 이 화면에 맞는 데이터를 뿌려줄 이유가 없었습니다.
uiState 역시 사용하지 않고있으니 추가하는것도 고려해볼 수 있는 부분입니다.
고려할 수 있는 부분은 다양하지만 가장 쉬운 방법은
val value = remember(key/* option */) { mutableStateOf(list.join) }
정도만 하더라도 큰 문제가 없습니다.
domain을 수정하는 케이스
화면별로 조금씩 다른 형태로 노출합니다. 그런데 domain을 수정하면 화면 별로 2가지 타입(또는 이후에 추가될 수 있는 타입) 까지 고려할 이유는 없습니다. ViewModel을 활용해서 그걸 끈어내기위함입니다. 이야기하신 도메인을 수정해서 뭔가를 한다고 하면, 추후 새로운 화면 타입이 들어오고, 다른 방식으로 화면 노출을 한다고하면 도메인을 수정도 고려가 될것이고요.
그렇지 않게 하려고 ViewModel을 활용해서 뷰에 맞는 화면의 데이터를 만들 수 있는데, 수정 범위가 너무 넓어집니다. 데이터가 변경되어지지 않는 이상은 수정이 필요하지 않아보입니다.
UiState를 적용
위젯이라서 ViewModel을 사용하지 않을순 있습니다. 하지만 사용할수도 있을것 같아요. Activity의 AAC-ViewModel 상속 받는 구조가 아닌 그냥 위젯용 ViewModel도 만들 수 있는것이라고 충분히 고려할 수 있어보입니다.
개인적으로 생각한 가장 쉬운 방법
remember만 활용해 바로 처리하는 방법입니다. 어차피 리컴포지션이 발생하면 join이 있다면 join을 다시 구동합니다. 그런 다음 내부 연산에 따라 skip 됩니다. 이때 join의 비용을 줄이는게 목적이었습니다.
이미 join 되어있다면 skip만 계산해서 내부적으로 처리되어질것이기에 불필요한 연산이 줄어드는 효과를 가질 수 있게됩니다.
추가로 이 코드에서는 derivedStateOf를 사용할 이유는 특별히 없어보입니다.
There was a problem hiding this comment.
remember만 활용해 바로 처리하는 방법입니다. 어차피 리컴포지션이 발생하면 join이 있다면 join을 다시 구동합니다. 그런 다음 내부 연산에 따라 skip 됩니다. 이때 join의 비용을 줄이는게 목적이었습니다.
이미 join 되어있다면 skip만 계산해서 내부적으로 처리되어질것이기에 불필요한 연산이 줄어드는 효과를 가질 수 있게됩니다.추가로 이 코드에서는 derivedStateOf를 사용할 이유는 특별히 없어보입니다.
오, 확실히 그러네요
There was a problem hiding this comment.
@taehwandev
기본적으로 프로젝트 내에서 한 데이터 클래스로 끝내는 uiState를 사용하는 것과, 이후에 session 이외의 것도 표시하거나, 추가적으로 복잡한 처리가 있을 경우, 뷰모델을 추가할 것을 고려해서 uiState로 수정했습니다.
| val session: Session, | ||
| ) { | ||
| val speakerLabel: String | ||
| get() = session.speakers.joinToString { it.name } |
There was a problem hiding this comment.
get() 이라서 매번 join 하던건 동일할것 같네요.
by lazy {}로 하면 좋을것 같네요.
There was a problem hiding this comment.
@taehwandev
지적하신 내용을 수정했습니다.
Issue
Overview (Required)
Links
Screenshot
Detail
Screen_recording_20240527_210755.mp4
Screen_Recording_20240509_224922.mp4
Widget