[feat] 4주차 과제 #8
Conversation
t1nm1ksun
requested review from
chattymin,
tunaunnie,
jihyunniiii and
angryPodo
and removed request for
a team
|
|
||
| return characterTypesCount >= 3 | ||
| private fun isValidateHobby(hobby: String): Boolean { | ||
| return hobby.length in 1..8 |
There was a problem hiding this comment.
저는 Result에 해당하는 부분 data class를 아예 외부에 분리해서 짰었는데 뭔가 기능상에 차이점이 있나요?? 이 방식이 가독성 측면에선 훨씬 좋은 것 같아보이네요
There was a problem hiding this comment.
이렇게 Result를 아예 밖에 분리하는 거랑 앞처럼 본문 안에 써주는거랑 차이점이 있는지 궁금합니다!!
|
|
||
| interface UserService { | ||
| @POST("/user") | ||
| suspend fun postSignup(@Body requestDto: RequestSignUpDto): Response<ResponseSignUpDto> |
There was a problem hiding this comment.
응답 타입이 늘 ResponseSignUpDto로 설정되어 있는데, 응답이 실패한 경우에는 어떻게 FailedDTO를 반환할 수 있는 건가요?? 저도 성공/실패 DTO를 구분했었는데, 이 부분에 반환값을 늘 성공한 경우 DTO로 줘도 되는 건지 헷갈리더라구요 ..
| val response = userService.getUserHobby(userInfoLocalDataSource.accessToken) | ||
| if (response.isSuccessful) { | ||
| Log.d("ㅋㅋ", "Status code: ${response.code()}") | ||
| response.body()?.result?.hobby ?: "내 취미 내놔!!" |
| } | ||
| } catch (e: Exception) { | ||
| Log.e("MyPageViewModel", "Exception while fetching hobby: ${e.localizedMessage}", e) | ||
| "에러났지렁이" |
| _user.value.email = email | ||
| private fun loadUserData() { | ||
| viewModelScope.launch { | ||
| val nickname = userInfoLocalDataSource.nickname |
There was a problem hiding this comment.
마이페이지 Screen(뷰)쪽에서는 늘 마이페이지ViewModel에서 데이터를 가져오는거고, 마이페이지ViewModel 안에서는 늘 최신 데이터(?)를 이 userInfoLocalDataSource에서 가져오는 건가요?? (약간 전역처럼..) 저는 코드를 짜다보니까 UserViewModel이랑 MypageViewModel을 사용하는 게 점점 짬뽕이 돼서 어떻게 기능상의 분리를 해야 하는건지 헷갈리더라구요..! 이 userInfoLocalDataSoucre가, 각 페이지의 ViewModel이 유저 관련 제일 최신 데이터를 가져오는 최상위 뷰모델(?) 전역변수? 뷰모델의 뷰모델..? 느낌으로 이해해도 되는 건가요??ㅜ
There was a problem hiding this comment.
userInfoLocalDataSource는 SharedPreferences 관련 로직들을 담고 있는 부분인데요! SharedPreferences 이 친구가 어떤 역할을 해주는 친구인지 찾아봅시다!
| @@ -54,8 +53,8 @@ fun SignInScreen( | |||
|
|
|||
| val snackBarHostState = remember { SnackbarHostState() } | |||
| val coroutine = rememberCoroutineScope() | |||
| val loginResult by signInViewModel.isLoginSuccessful.collectAsState() | |||
There was a problem hiding this comment.
collectAsState의 단점을 찾아보시고, 해결하는 방법을 적용해보시면 좋을 것 같습니다 :)
OB분이시니 직접적인 답은 드리지 않을거에요!
힌트는 LiveData와 Flow의 차이점을 생각해보세용 ㅎㅎ
| LaunchedEffect(loginResult) { | ||
| loginResult?.let { | ||
| if (it) { | ||
| snackBarHostState.showSnackbar(message = context.getString(R.string.sign_in_success)) | ||
| delay(300) | ||
| navigateToMyPage(userEmail.value) | ||
| } else { | ||
| snackBarHostState.showSnackbar(message = context.getString(R.string.sign_in_failed)) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
화면을 이동시키는 로직을 UI가 직접적으로 호출하고 있어요.
loginResult에 따라 ViewModel에 알려주고, event를 발생시켜 snackBar 호출 및 화면 이동을 하면 좋을 것 같아요.
| Log.e("ㅋㅋ", "Status code: ${response.code()} and error code: $errorCode") | ||
| _isLoginSuccessful.value = false | ||
| } | ||
| } catch (e: Exception) { |
There was a problem hiding this comment.
try-catch 문법을 사용한다면 Exception을 명시해주는게 좋아요.
그래야 추후 디버깅을 할 때에도, 다른사람이 내 코드를 볼 때에도 왜 try-catch문을 사용했는지 알려줄 수 있어요
| private val _signUpResult = MutableStateFlow<Result<Unit>?>(null) | ||
| val signUpResult: StateFlow<Result<Unit>?> = _signUpResult |
There was a problem hiding this comment.
Effective Kotlin에서 나와있는 내용중 일부에요
Unit?을 리턴하는 것은 권장되지 않아요.
그리고 아래 사용법을 보니 상태를 저장한다기 보다는 event를 발생시키는 용도로 사용하고자 하시는 것 같은데 StateFlow와 SharedFlow의 사용법 차이를 찾아보시고 적용하면 좋을 것 같아요,.
| private fun isValidEmail(email: String): Boolean { | ||
| return email.length in 1..8 | ||
| } | ||
|
|
||
| val characterTypesCount = | ||
| listOf(lowercase > 0, uppercase > 0, digit > 0, specialChar > 0).count { it } | ||
| private fun isValidPassword(password: String): Boolean { | ||
| return password.length in 1..8 | ||
| } | ||
|
|
||
| return characterTypesCount >= 3 | ||
| private fun isValidateHobby(hobby: String): Boolean { | ||
| return hobby.length in 1..8 |
There was a problem hiding this comment.
유효한 길이라는 규칙은 유동적으로 수정될 수 있고, 이렇게 숫자로 되어있다면 추후 파악하기 힘들 수 있어요.
상수로 빼두고 호출해서 사용한다면 더욱 좋을 것 같네요!
| // Network | ||
| implementation(platform(libs.okhttp.bom)) | ||
| implementation(libs.okhttp) | ||
| implementation(libs.okhttp.logging.interceptor) | ||
| implementation(libs.retrofit) | ||
| implementation(libs.retrofit.kotlin.serialization.converter) | ||
| implementation(libs.kotlinx.serialization.json) | ||
| implementation(libs.retrofit.v2110) | ||
| implementation(libs.converter.gson) |
There was a problem hiding this comment.
bundle을 사용해보셔도 좋을 것 같슴다 ~
|
|
||
|
|
|
|
||
|
|
| @Serializable | ||
| data class ResponseLoginDto( | ||
| @SerialName("result") | ||
| val result: Result | ||
| ) { |
There was a problem hiding this comment.
해당 부분이 계속 공통되는 것 같은데 BaseResponse를 사용하시는 것도 방법이 될 것 같네요!
| private val retrofit: Retrofit by lazy { | ||
| Retrofit.Builder() | ||
| .baseUrl(BASE_URL) | ||
| .addConverterFactory(GsonConverterFactory.create()) |
There was a problem hiding this comment.
왜 Gson을 사용하신 건지 여쭤봐두 되나염?
| import kotlinx.coroutines.flow.MutableStateFlow | ||
| import kotlinx.coroutines.flow.StateFlow | ||
| import kotlinx.coroutines.launch | ||
| import org.sopt.and.data.datalocal.datasource.UserInfoLocalDataSource |
There was a problem hiding this comment.
클린 아키텍처에서 의존성의 방향에 대해서 생각해봅시다! 어떻게 수정되어야 할까요?
| } | ||
| } catch (e: Exception) { | ||
| Log.e("MyPageViewModel", "Exception while fetching hobby: ${e.localizedMessage}", e) | ||
| "에러났지렁이" |
| _user.value.email = email | ||
| private fun loadUserData() { | ||
| viewModelScope.launch { | ||
| val nickname = userInfoLocalDataSource.nickname |
There was a problem hiding this comment.
userInfoLocalDataSource는 SharedPreferences 관련 로직들을 담고 있는 부분인데요! SharedPreferences 이 친구가 어떤 역할을 해주는 친구인지 찾아봅시다!
| ) { | ||
|
|
||
| myPageViewModel.updateUserEmail(email = email) | ||
| val user by myPageViewModel.user.collectAsState() |
There was a problem hiding this comment.
collectAsState 말고 어떤 걸 쓰면 좋을까요?
Related issue 🛠
Work Description ✏️
Screenshot 📸
2024-11-14.8.32.01.mov
Uncompleted Tasks 😅
To Reviewers 📢
다들 합세 화이팅~!