🚀 3단계 - GitHub(UI 상태) #56
Conversation
…ainer 속성으로 대체 및 Color White 제거
| data class RepositoryUiState( | ||
| val id: Int, | ||
| val fullName: String, | ||
| val description: String, | ||
| ) |
There was a problem hiding this comment.
UiState가 RepositoryModel를 들고있어도 될텐데요! 이 점은 어떻게 생각하시나요?
There was a problem hiding this comment.
RepositoryUiState가 바로 RepositoryModel을 들고 있을수도 있겠지만 이 경우 Data Layer에서만 활용하기 위해RepositoryModel에 데이터 값이 새로 생긴다고 가정했을때 RepositoryUiState에서는 불필요한 속성을 같이 가져가기 때문에 최소한의 필요 데이터만 남기기위해 나누는게 맞다는 판단이 듭니다. 리뷰어님은 어떻게 구조를 가져가시나요?
There was a problem hiding this comment.
맞아요. 말씀하신 내용도 공감됩니다.
저는 개인적으로는 불필요한 속성이 있더라도 그것을 사용하는 것은 클라이언트(UI)의 자유라고 생각하는데요.
뷰모델이 UI에서 어떻게 그려야 할지 직접적으로 의존하는 것처럼 보여서입니다.
가령 화면에 이름, 나이, 성별을 출력하는데 나중에 디자이너가 밑에 주소도 출력해주세요! 라고 한다면 개발자가
뷰모델에 이런 테스트를 짜야 할 필요가 있나? 싶어서요
뷰모델은 유저 정보를 받으면, 주소를 만들어 내야 한다.
뭐 그런 의미로 UI 레이어에 모델을 따로 만들기도 하니 이 내용은 정답은 없는 주제같습니다!
| val repositoryList by viewModel.repositoryList.collectAsState() | ||
| val repositoryUiState by viewModel.repositoryUiState.collectAsStateWithLifecycle() | ||
|
|
||
| val snackbarHostState = remember { SnackbarHostState() } |
There was a problem hiding this comment.
그럼 여기서 스낵바 호출 로직 테스트는 동적 화면에서만 가능하니 동적 화면에는 관련 테스트 코드만 작성하고 나머지 데이터 조회 같은 테스트는 Viewmodel 단위 테스트를 따로 만들어 진행해야 하는 것이 옳은 것인지에 대한 고민이 생겼습니다.
테스트 코드를 평소에 많이 작성해보지 않다보니 테스트 영역 분할에 대한 고민이 많이 생기는데 리뷰어님이 보시기엔 테스트 영역 분할을 어떻게 가져가는게 좋을까요..?
스낵바가 동적 화면에서만 테스트가 가능하다는 이유는 어떤건가요?
UI 테스트 환경에서도 코루틴을 사용할 수 있습니다!
동일하게 시나리오 관점에서 바라본다면... 이렇게 바라볼 수 있을 것 같은데요!
OOO라는 메시지를 가진 스낵바가 보여진다.
스낵바의 액션 버튼을 누르면 스낵바가 닫힌다.
이 외에도 snackBarHostState.currentSnackbarData?.dismiss() 와 같은 API를 직접 제어할 수 있는데요.
참고해볼 수 있을 것 같아요.
먼저 미션을 마무리 하신 다른 분의 코드를 참고해보시면 이해하는데 더욱 도움이 될 것 같네요!!
#50
There was a problem hiding this comment.
아 제가 너무 내부쪽으로만 치중해서 생각했던 것 같습니다ㅜㅠ 확실히 외부에서 선언해서 스낵바가 호출되는 테스트를 할 수 있는데 말이죠 허헣;; 바로 추가해보겠습니다!
| LaunchedEffect(Unit) { | ||
| viewModel.errorFlow.collectLatest { message -> | ||
| val result = snackbarHostState.showSnackbar( | ||
| message = snackbarRetryMesssage, | ||
| actionLabel = retryActionLabel, | ||
| duration = SnackbarDuration.Indefinite | ||
| ) | ||
| if (result == SnackbarResult.ActionPerformed) { | ||
| viewModel.onRetry() | ||
| } | ||
| } |
There was a problem hiding this comment.
LaunchedEffect에 들어가는 key값이 어떤 역할을 하는지 아시나요!?
가령 viewModel 인스턴스가 바뀌더라도 이전 뷰모델을 계속 참조하는 문제가 발생할 수 있는데요.
아래 코드가 어떤 차이가 있는지 살펴보시면 됩니다!
LaunchedEffect(viewModel) { ... }There was a problem hiding this comment.
key의 변동이 생긴다면 LaunchedEffect 내부의 코드가 재실행되는걸로 알 고 있습니다.
작성해주신 코드 같은 경우는 viewModel이 다른 인스턴스로 대체된다거나 하면 내부 코드가 동작할 것 같습니다.
일단 viewModel이 대체될 일이 없다고 생각하여 이렇게 작성하였는데 그래도 viewModel 관련 로직이 내부에 들어가있다면 키로 지정해주는게 좋은 방식일가요?
There was a problem hiding this comment.
네 논리적으로는 뷰모델이 다시 만들어질 경우가 없으니까 크게 중요한 부분은 아닙니다.
다만 습관적으로 Unit을 넣다보면 나중에 실수하거나 정말 바뀌어야 하는 상황에 놓칠 수가 있어서요.
저는 습관이 될 필요가 있다면 챙기는 방향으로 하는 것이 좋다고 생각해서요.
| val snackbarRetryMesssage = stringResource(R.string.snackbar_retry_error_message) | ||
| val retryActionLabel = stringResource(R.string.retry_action_label) |
There was a problem hiding this comment.
기존 하드코딩 문구 stringresource로 대체
근데 저렇게 쓰는 것 보다 그냥 LocalContext.current를 활용해서 바로 불러오는게 나았을까요?
음 스낵바에 들어가는 문구는 편하신 방법을 사용해도 될 것 같아요!
저는 지금 구현해주신 방법처럼 사용할 것 같아요.
| LaunchedEffect(Unit) { | ||
| viewModel.errorFlow.collectLatest { message -> | ||
| val result = snackbarHostState.showSnackbar( | ||
| message = snackbarRetryMesssage, | ||
| actionLabel = retryActionLabel, | ||
| duration = SnackbarDuration.Indefinite | ||
| ) | ||
| if (result == SnackbarResult.ActionPerformed) { | ||
| viewModel.onRetry() | ||
| } | ||
| } |
There was a problem hiding this comment.
조회 오류시 스낵바를 호출하여 재시도를 통해 다시 데이터를 조회해 오는 로직을 구성하였습니다.
이때 StateHoisting을 위해 스낵바 관련 상태 선언 및 호출 로직을 stateful screen에서 관리하고 stateless screen에는 snackbarHostState를 파라미터로 전달하여 Scaffold에 지정해주 었습니다.
이러한 형태의 구성도 괜찮은지 궁금합니다!
네, 스낵바 메시지가 에러를 트리거로 발생하기 때문에 적절한 방법을 잘 선택 해주신 것 같아요.
상태로 처리하는 방법도 있지만 이건 사람마다 다른 것 같더라구요.
class SnackbarMessage(val message: String)
var snackbarMessage: String? by remember { mutableStateOf(null) }
LaunchedEffect(snackbarMesage) { showSnackBar(...); snackbarMessage = null; }| viewModelScope.launch { | ||
| _errorFlow.emit(message) |
There was a problem hiding this comment.
컴포즈와 관련된 내용은 아닙니다~
SharedFlow를 코루틴이 아닌 곳에서도 emit 할 수 있는 방법을 아시나요? tryEmit과 emit의 차이?
There was a problem hiding this comment.
emit 같은 경우 구독자가 값을 받을 준비가 될때까지 suspend를 하여 비동기로 실행하야하지만 tryEmit 같은 경우에는 구독자가 없어도 즉시 값을 반환하여 비동기 실행이 필요 없는 것으로 알고 있습니다!
현재 상황에서는 굳이 비동기로 뺄 필요가 없을 것 같네요! tryEmit으로 수정해보겠습니다!
| sealed class UiState<out T> { | ||
| data object Empty : UiState<Nothing>() | ||
| data object Loading : UiState<Nothing>() | ||
| data class Success<T>(val data: T) : UiState<T>() | ||
| data class Failure(val meesage: String) : UiState<Nothing>() | ||
| } |
There was a problem hiding this comment.
개인적으로 나눠보고 싶은 이야기가 있어요~
UiState를 나누는 방법은 여러가지 전략이 있지만 특히 지금과 같이 UiState를 한번더 Wrapping해서 쓰는 점은 어떻게 바라보시나요?
만약 이렇게도 작성해도 똑같은 동작이 아닐까? 싶어서요.
sealed class GithubUiState {
data object Empty: ...
data class Success(val repositories: List<Repository>)
data class Failure(...)
}There was a problem hiding this comment.
아 제가 이부분은 UiState를 다양한 타입으로 공통으로 쓸것을 염두해두고 작업해보았습니다! 위처럼 작성해도 똑같이 동작하지만 추후 확장성을 고려해서 Wrapping해서 쓰는 방식을 사용해보았는데 혹시 현업에서는 UiState를 종류별로 다 두는 편일가요?
There was a problem hiding this comment.
아뇨 Ui 상태를 어떻게 만드는지는 검색해보시면 다양한 레퍼런스가 있기는 합니다.
지금 구조에서는 Ui 상태를 객체로 만드려면 UiState<XXUiState> 형태로 이중적으로 만들어질 것 같아서요.
저 개인적으로는 TopLevel UI 상태는 data class로 만듭니다. (취향입니다)
|
리뷰어님 안녕하세요! |
🚀 3단계 - GitHub(UI 상태)
안녕하십니까 리뷰어님!
3단계 미션이 완료되어 PR 요청드립니다!
2단계 미션에서 말씀해주신 부분도 열심히 개선해서 반영해보았습니다.
이번 3단계 미션을 수행하며 궁금했던점은 아래와 같습니다.
스낵바 오류 처리 관련 90b0c80
조회 오류시 스낵바를 호출하여 재시도를 통해 다시 데이터를 조회해 오는 로직을 구성하였습니다.
이때 StateHoisting을 위해 스낵바 관련 상태 선언 및 호출 로직을 stateful screen에서 관리하고 stateless screen에는 snackbarHostState를 파라미터로 전달하여 Scaffold에 지정해주 었습니다.
이러한 형태의 구성도 괜찮은지 궁금합니다!
기존 하드코딩 문구 stringresource로 대체 911087d
기존 하드코딩된 문구들을 string.xml에 선언하여 불러오는것으로 대체하였습니다.
여기서 스낵바 메시지 같은 경우에는 호출부 내부에서 stringResource를 활용할 수 없어 저렇게 상단에 메시지를 미리 선언해두고 사용하였습니다.
근데 저렇게 쓰는 것 보다 그냥 LocalContext.current를 활용해서 바로 불러오는게 나았을까요?
테스트 코드 정적, 동적 화면으로 분할 5ef8e7f, 17833de
테스트 코드를 정적 화면과 동적 화면으로 분할해서 작성해보았습니다.
근데 여기서 동적 화면에서 작성한 테스트 코드가 정적 화면 테스트 코드와 겹치는 부분이 많습니다.
물론 로직은 다르지만 이렇게 테스트 코드를 작성해도 괜찮을가요?
그리고 동적 화면 테스트 코드를 작성하면서 고민이 되었던 부분이 있습니다.
지난 시간 수업에서 듣기를 정적화면과 viewmodel 테스트를 나눠야 한다는 말이 기억납니다.
그럼 여기서 스낵바 호출 로직 테스트는 동적 화면에서만 가능하니 동적 화면에는 관련 테스트 코드만 작성하고 나머지 데이터 조회 같은 테스트는 Viewmodel 단위 테스트를 따로 만들어 진행해야 하는 것이 옳은 것인지에 대한 고민이 생겼습니다.
테스트 코드를 평소에 많이 작성해보지 않다보니 테스트 영역 분할에 대한 고민이 많이 생기는데 리뷰어님이 보시기엔 테스트 영역 분할을 어떻게 가져가는게 좋을까요..?