refactor: ViewModel의 LiveData를 Flow로 변경 #695
Open
Conversation
This file contains 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
aprilgom
approved these changes
Oct 11, 2024
| @@ -13,11 +10,17 @@ import com.mulberry.ody.presentation.address.listener.AddressListener | |||
| import com.mulberry.ody.presentation.address.model.AddressUiModel | |||
| import com.mulberry.ody.presentation.address.model.toAddressUiModels | |||
| import com.mulberry.ody.presentation.common.BaseViewModel | |||
| import com.mulberry.ody.presentation.common.MutableSingleLiveData | |||
| import com.mulberry.ody.presentation.common.SingleLiveData | |||
| } | ||
| } | ||
|
|
||
| fun withdrawAccount() { | ||
| viewModelScope.launch { | ||
| startLoading() | ||
| loginRepository.withdrawAccount() | ||
| .onSuccess { | ||
| _loginNavigateEvent.setValue(LoginNavigatedReason.WITHDRAWAL) | ||
| .suspendOnSuccess { |
There was a problem hiding this comment.
suspendOnSuccess가 여기에 필요한 거였나요 😮
| val hasAddressSearchKeyword: StateFlow<Boolean> = | ||
| addressSearchKeyword.map { it.isNotEmpty() }.stateIn( | ||
| scope = viewModelScope, | ||
| started = SharingStarted.WhileSubscribed(5000), |
There was a problem hiding this comment.
configuration change 대응 코드인가요??
kimhm0728
requested changes
Oct 11, 2024
| val navigatedReason: SingleLiveData<LoginNavigatedReason> get() = _navigatedReason | ||
| private val _navigatedReason: MutableSharedFlow<LoginNavigatedReason> = | ||
| MutableSharedFlow() | ||
| val navigatedReason: SharedFlow<LoginNavigatedReason> get() = _navigatedReason.asSharedFlow() |
There was a problem hiding this comment.
Suggested change
| val navigatedReason: SharedFlow<LoginNavigatedReason> get() = _navigatedReason.asSharedFlow() | |
| val navigatedReason: SharedFlow<LoginNavigatedReason> get() = _navigatedReason |
asSharedFlow()가 없어도 SharedFlow로 형변환되는 걸로 알고 있는데 명시해준 이유가 있을까요?
| val actual = viewModel.isValidInfo.getOrAwaitValue() | ||
| assertThat(actual).isTrue | ||
| val actual = viewModel.isValidInfo.value | ||
| assertThat(actual).isTrue() |
There was a problem hiding this comment.
다른 테스트에서는 괄호 없이 isTrue, isFalse로만 사용하고 있는데 여기도 통일성을 위해 괄호빼면 어떨까요?
| @@ -159,11 +158,11 @@ class MeetingCreationViewModelTest { | |||
|
|
|||
| // when | |||
| viewModel.meetingDate.value = LocalDate.of(2023, 7, 28) | |||
| viewModel.meetingHour.value = 18 | |||
| viewModel.meetingHour.value = 17 | |||
| @@ -109,7 +119,7 @@ class MeetingCreationViewModel | |||
| } | |||
|
|
|||
| private fun createMeetingCreationInfo(): MeetingCreationInfo? { | |||
| val name = meetingName.value ?: return null | |||
| val name = meetingName.value.takeIf { it.isNotEmpty() } ?: return null | |||
There was a problem hiding this comment.
Suggested change
| val name = meetingName.value.takeIf { it.isNotEmpty() } ?: return null | |
| val name = meetingName.value.ifBlank { return null } |
이렇게 변경하면 더 직관적이지 않을까요?
Comment on lines
+52
to
+53
| val meetingHour = MutableStateFlow<Int?>(null) | ||
| val meetingMinute = MutableStateFlow<Int?>(null) |
There was a problem hiding this comment.
nullable 타입은 최대한 지양하면 좋을 것 같은데 createMeetingCreationInfo()에서 null인 경우 return하기 위해서 이렇게 작성하신 건가요??
Suggested change
| val meetingHour = MutableStateFlow<Int?>(null) | |
| val meetingMinute = MutableStateFlow<Int?>(null) | |
| val meetingHour = MutableStateFlow<Int>(-1) | |
| val meetingMinute = MutableStateFlow<Int>(-1) |
이렇게 변경하고 createMeetingCreationInfo()에서 -1인지 검사하면 어떨까요? _inviteCode 등 다른 프로퍼티도 마찬가지입니다~
| val isEmptyAddresses: StateFlow<Boolean> = | ||
| addresses.map { it.isEmpty() }.stateIn( | ||
| scope = viewModelScope, | ||
| started = SharingStarted.WhileSubscribed(5000), |
There was a problem hiding this comment.
5000 상수로 빼면 어떨까요?? 다른 ViewModel도 마찬가지입니다!!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
🚩 연관 이슈
close #640
📝 작업 내용
🏞️ 스크린샷 (선택)
🗣️ 리뷰 요구사항 (선택)
Flow로 다 마이그레이션 하면 MutableSingleLiveData 지워야 합니다!