-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Feat] 7주차 과제 완료 #14
base: develop
Are you sure you want to change the base?
[Feat] 7주차 과제 완료 #14
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MVI가 참 어렵죠ㅠㅠ 이번주도 구현하시느냐 이번 기수 과제하느냐 고생 너무 많으셨습니다!!
fun logIn() { | ||
setState { copy(isLoading = true) } | ||
viewModelScope.launch { | ||
val state = uiState.value | ||
val result = userRepository.postUserLogin( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기에 상태 업데이트와 비지니스 로직이 섞여있는 것 같아용 약간 분리해줘도 좋을 것 같은데요??
val username: String = "", | ||
val password: String = "", | ||
val isLoading: Boolean = false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요기서는 왜 모든 값에 기본값을 제공해주신걸꺼용?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
너무 잘 하셔서 새롭게 배운 부분에 커멘트 남겼습니다! 고생하셨습니다!! 앱잼 파이팅!!
when (effect) { | ||
is SignUpSideEffect.ShowToast -> { | ||
context.showToast(effect.message) | ||
} | ||
} | ||
|
||
is RegisterState.Failure -> { | ||
when (registerState.code) { | ||
"00" -> context.showToast(R.string.sign_up_error) | ||
"01" -> context.showToast(R.string.sign_up_eight) | ||
else -> {} | ||
is SignUpSideEffect.NavigateToLogin -> { | ||
navController.navigate("login") { | ||
popUpTo(0) { inclusive = true } | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
확실히 이전에 비해 훨씬 더 깔끔해졌네요!
abstract class BaseViewModel<State : UiState, SideEffect : UiSideEffect, Event : UiEvent> : | ||
ViewModel() { | ||
|
||
private val initialState: State by lazy { createInitialState() } | ||
abstract fun createInitialState(): State | ||
|
||
private val _uiState = MutableStateFlow(initialState) | ||
val uiState: StateFlow<State> = _uiState.asStateFlow() | ||
|
||
private val _sideEffect = MutableSharedFlow<SideEffect>() | ||
val sideEffect: SharedFlow<SideEffect> = _sideEffect.asSharedFlow() | ||
|
||
fun setState(reduce: State.() -> State) { | ||
_uiState.value = _uiState.value.reduce() | ||
} | ||
|
||
fun setSideEffect(builder: () -> SideEffect) { | ||
viewModelScope.launch { _sideEffect.emit(builder()) } | ||
} | ||
|
||
fun setEvent(event: Event) { | ||
viewModelScope.launch { handleEvent(event) } | ||
} | ||
|
||
protected abstract suspend fun handleEvent(event: Event) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BaseViewModel을 두어서 하는 방식 좋네요!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다 ~
import org.sopt.and.presentation.signup.components.IdHobbyTextField | ||
import org.sopt.and.presentation.signup.components.SocialServiceLogIn | ||
import org.sopt.and.util.showSnackBar | ||
|
||
@Composable | ||
fun LogInScreen( | ||
navController: NavController, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
책임 분리를 위해서 navController 자체를 인자로 받기 보다는 이를 이용한 함수를 인자로 받는 게 좋을 것 같아요!
관련한 내용이 합동 세미나 피드백에 있으니 읽어보시면 좋을 것 같습니다 ~
|
||
override fun createInitialState(): LogInState = LogInState() | ||
|
||
override suspend fun handleEvent(event: LogInEvent) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
event와 sideEffect의 차이는 뭘까요?
Related issue 🛠
Work Description ✏️
Screenshot 📸
전 주차 화면과 동일합니다!
Uncompleted Tasks 😅
To Reviewers 📢
과제 제출 기간을 잊어버렸네요... 죄송합니다...!
뷰모델과 State, SideEffect, Event를 컴포넌트화 하여 재사용하고, 행동을 정의할 수 있다는 것에 screen과 viewModel의 많은 코드 감소를 느낄 수 있었습니다! 본 과제 작동 방식이 맞는지 잘 몰라서 피드백 주시면 감사드립니다!