7주차 과제 제출 #12
7주차 과제 제출 #12
Conversation
| private val _state = MutableStateFlow(SignUpState()) | ||
| val state: StateFlow<SignUpState> get() = _state | ||
|
|
||
| private val _intentChannel = Channel<SignUpIntent>(Channel.UNLIMITED) |
There was a problem hiding this comment.
저는 사용해 본적이 없는데.. Channel에 대해 잘 사용하신 것 같아요!
궁금하여 찾아보니, Intent가 발생할 때마다 ViewModel은 해당 Intent를 받아서 처리하고, 새로운 상태를 생성하는 과정에서 Channel이 사용에 효과적이란 점을 배웠습니다!
There was a problem hiding this comment.
[3]
오 .. 저도 Channel 이라는 건 처음보네요. Intent 를 받아서 새로운 상태를 생성해주는 역할이라면 Reducer 와 비슷한 역할을 하는건가요?
There was a problem hiding this comment.
오호! Contract라는 파일 안에 Intent와 State을 넣어 구현하셨군요! 저는 각 Intent, State를 별도의 파일로 분리하였거든요. 지금처럼 관련 코드가 모여있어서 편할 수 있겠네요!
There was a problem hiding this comment.
MVI 많이 어려우셨을텐데, 수고하셨습니다!!
코드를 쭉 보니 아직은 살짝 난잡한 느낌이 있는 것 같아요
- 남아있는 콜백함수들을 코루틴으로 변경해봅시다!
- 토스트를 띄운다던지 어떤 로직을 실행할 때 그 로직을 발생시키는 원인에 대해서 다시 한번 생각해보면 좋을 것 같습니다! 재원님 코드를 예로 들자면 현재 스크린에서 successMessage나 errorMessage가 들어와서 null이 아니게 되면? 그때 토스트를 띄우게 설계되어있는 것 같은데 그 전에 이 메시지들에 값이 들어왔다는건 어떤 것을 의미할까요? 서버통신의 성공/실패 여부와 같은 상태가 그 원인일거라 생각합니다. 토스트를 띄우게 되는 궁극적인 이유가 이거라면, 로직도 이것을 기준으로 설계하면 좋지 않을까하는 개인적인 의견입니다! 한번 고민해보시면 도움이 될 것 같아요😀
| val authRepository = AuthRepositoryImpl(ServicePool.apiService) | ||
| val factory = ViewModelFactory(authRepository, context) | ||
| val viewModel: SignInViewModel = | ||
| androidx.lifecycle.viewmodel.compose.viewModel(factory = factory) |
There was a problem hiding this comment.
[3]
이 친구는 import를 살짝 손봐볼까요? ㅎㅎ
| LaunchedEffect(state.token) { | ||
| state.token?.let { token -> | ||
| context.getSharedPreferences("auth", Context.MODE_PRIVATE) | ||
| .edit() | ||
| .putString("token", token) | ||
| .apply() | ||
| navigateToMain() | ||
| } | ||
| } |
There was a problem hiding this comment.
[2]
토큰은 사실상 Screen 에서 사용할 부분이 없기에, ViewModel에서 처리할 수 있도록 개선해보면 어떨까싶습니다!
There was a problem hiding this comment.
[2]
어라라.. 아직 콜백이 남아있네요..! 우리 Coroutine 으로 변경하기로 했던 것 같은데!!
| Spacer(modifier = Modifier.height(24.dp)) | ||
|
|
||
| SignInSignUpButton( | ||
| text = "로그인", |
There was a problem hiding this comment.
현업에서는 라벨이나 여러 텍스트 값들 그대로 쓰기보다는 string.xml 에서 관리하거나 따로 빼서 관리하는데 그러한 이유는 어떤걸까요?
| } | ||
| } | ||
|
|
||
| LaunchedEffect(state.errorMessage) { |
There was a problem hiding this comment.
에러메시지를 토스트로 띄우는 코드의 경우 sideEffect로 사용하는게 좀 더 좋아보여요
토스트 메시지, 화면 이동 과 같이 화면 상황이 아닌 부가적인 효과를 관리하려구 사이드 이펙트를 쓰는거니까 state.erroMessage보단 SignInSideEffect를 하나 만들어 사이드 이펙트로 관리하는 것이 MVI 패턴에 좀 더 맞는 방법같아요
| } | ||
| } | ||
|
|
||
| private fun signIn() { |
There was a problem hiding this comment.
객체 지향의 원칙을 따르려면 함수는 한번에 하나의 역할을 해야합니다.
user이름에 대해 검증하는 것과 로그인 통신을
각각의 메소드로 분리해보는 건 어떨까요?
| _state.update { it.copy(isLoading = true) } | ||
|
|
||
| val request = LoginRequestDto(username, password) | ||
| authRepository.login(request).enqueue(object : Callback<LoginResponseDto> { |
There was a problem hiding this comment.
result를 사용하면 통신에 있어서 성공 실패에 대해 좀 더 간결한 코드를 사용할 수 있답니다.
현재는 코드가 너무 길어보여요.
| private val _state = MutableStateFlow(SignUpState()) | ||
| val state: StateFlow<SignUpState> get() = _state | ||
|
|
||
| private val _intentChannel = Channel<SignUpIntent>(Channel.UNLIMITED) |
| val authRepository = AuthRepositoryImpl(ServicePool.apiService) | ||
| val factory = ViewModelFactory(authRepository, context) | ||
| val viewModel: MypageViewModel = androidx.lifecycle.viewmodel.compose.viewModel(factory = factory) | ||
|
|
There was a problem hiding this comment.
추후 hilt를 사용한 의존성 주입을 해보시는 걸 추천드려요!
| companion object { | ||
| private const val TAG = "AuthRepositoryImpl" | ||
| } | ||
|
|
There was a problem hiding this comment.
companion object는 가장 하단에 작성해주시는 것이 컨벤션입니다.
| val hobby: String = "", | ||
| val isLoading: Boolean = false, | ||
| val successMessage: String? = null, | ||
| val errorMessage: String? = null |
There was a problem hiding this comment.
intent와 sideEffect의 차이에 대해 고민해보시고, 어떤 값들을 상태로 어떤 값들을 sideEffect로 관리하면 좋을지 생각해보시면 좋을 . 것 같네요.
|
|
||
| val state by viewModel.state.collectAsState() |
There was a problem hiding this comment.
collectAsStateWithLifecycle로 관리하는 것이 더 좋을 것 같네요. (왜일까요?)
Related issue 🛠
Work Description ✏️
Screenshot 📸
KakaoTalk_20241216_143253112.mp4
To Reviewers 📢
많이 틀릴 수도 있겠지만 최대한 해봤습니다ㅎ