[Feat] 4주차 과제 완료 #10
Open
[Feat] 4주차 과제 완료 #10
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
- Permission Internet 추가 - gradle Third Party 추가
- data 폴더 생성 및 회원가입 DTO 구현
- 통신 사용을 위한 interceptor, retrofit, interface 구현
- Body 객체로 request 타입 변경
- 통신 작업 추가
- hobby textField 추가 - id와 hobby textField 공통 컴포넌트화
- id, hobby placeHolder 인자 전달 방식으로 변경
- hobbyState 필드 비어있지 않을 때 회원가입 활성화
- my-hobby api 호출 추가 - SharedPreference id 저장 삭제 - TokenInterceptor에 Header 구현 방식 추가
hyoeunjoo
requested changes
Nov 17, 2024
Comment on lines
+16
to
+23
| @Serializable | ||
| data class ResponseUserRegisterDto( | ||
| @SerialName("result") | ||
| val result: ResponseRegisterResultDto? = null, | ||
| @SerialName("code") | ||
| val code : String? = null | ||
| ) | ||
|
|
There was a problem hiding this comment.
이 부분은 계속해서 반복되고 있네용 이런 경우는 BaseResponse로 따로 형태를 정의해주고 ResponseDto를 BaseResponse로 감싸줬습니다! ㅎㅎ 이렇게 하면 같은 형태의 응답구조를 모든 Dto마다 따로 만들어 줄 필요가 없게 됩니다!
Comment on lines
+10
to
+21
|
|
||
| interface UserService { | ||
| @POST("/user") | ||
| fun postUserRegister( | ||
| @Body request: RequestUserRegisterDto | ||
| ): Call<ResponseUserRegisterDto> | ||
|
|
||
| @POST("/login") | ||
| fun postUserLogin( | ||
| @Body request: RequestUserLoginDto | ||
| ): Call<ResponseUserLoginDto> | ||
| } |
| IdTextField( | ||
| IdHobbyTextField( |
There was a problem hiding this comment.
이 부분은 조금 더 생각을 해봐야 하는 부분인 것 같아요.. (저도 아직 못고침)
만약 아래에 같은 형태의 텍스트 필드의 모양을 가진 mbti의 입력을 갖게 된다면? 그때부턴 이제 더이상 특정한 형태로 어딘가에 특별하게 쓰이지 않기 때문에 텍스트 필드 이름을 바꾸는게 좋겠져...
그냥 주저리 해봤슴다..ㅋ 별 내용은 아녀요
Comment on lines
+43
to
+68
| fun getUserHobby(){ | ||
| viewModelScope.launch { | ||
| myService.getMyHobby().enqueue(object: | ||
| Callback<ResponseMyHobbyDto> { | ||
| override fun onResponse( | ||
| call: Call<ResponseMyHobbyDto>, | ||
| response: Response<ResponseMyHobbyDto> | ||
| ) { | ||
| if (response.isSuccessful) { | ||
| val hobbyResponse = response.body() | ||
| _hobbyState.value = hobbyResponse?.result?.hobby | ||
| } | ||
| else { | ||
| val error = response.message() | ||
| Log.e("error", error.toString()) | ||
| } | ||
| } | ||
|
|
||
| override fun onFailure(call: Call<ResponseMyHobbyDto>, t: Throwable) { | ||
| Log.e("failure", t.message.toString()) | ||
| } | ||
|
|
||
| }) | ||
| } | ||
| } | ||
|
|
Comment on lines
32
to
34
| class UserViewModel : ViewModel() { | ||
| private var userData = UserData() | ||
| private val userService by lazy { ServicePool.userService } | ||
|
|
Comment on lines
+43
to
+46
| when (checkSignUpValue(username, password, hobby)) { | ||
| "idError" -> _authState.value = AuthState.Error(R.string.sign_up_error, AuthType.SIGNUP) | ||
| "passwdError" -> _authState.value = AuthState.Error(R.string.sign_up_paswd, AuthType.SIGNUP) | ||
| "hobbyError" -> _authState.value = AuthState.Error(R.string.sign_up_error_hobby, AuthType.SIGNUP) |
There was a problem hiding this comment.
명세서 보면 서버에서 8자이상인지 아닌지 검증을 해주고 있습니다! 데이터를 서버로 보낸 뒤에 서버 응답 에러코드를 통해서 에러핸들링을 해주시면 굳이 이 부분이 필요 없을거에요!
Comment on lines
+65
to
+69
| override fun onFailure(call: Call<ResponseUserRegisterDto>, t: Throwable) { | ||
| Log.e("failure", t.message.toString()) | ||
| } | ||
| }) | ||
|
|
sayyyho
approved these changes
Nov 17, 2024
Comment on lines
+7
to
+12
| // TokenInterceptor 에서 Header intercept | ||
| interface MyService { | ||
| @GET("/user/my-hobby") | ||
| fun getMyHobby( | ||
| ): Call<ResponseMyHobbyDto> | ||
| } |
There was a problem hiding this comment.
결국 이 API의 path가 user인데 클라이언트단에서 My 라는 관심사로 분리하신 이유가 궁금합니다
There was a problem hiding this comment.
저두 궁금해요! 서비스를 분리하시는 기준이 있으신가요?
| @@ -54,7 +55,7 @@ fun MyScreen(paddingValues: PaddingValues) { | |||
| ) | |||
| Spacer(modifier = Modifier.size(8.dp)) | |||
| Text( | |||
| text = "${SharedPreferenceManager.getUserId()}", | |||
| text = "${viewModel.hobbyState.value}", | |||
Comment on lines
+51
to
+53
| if (response.isSuccessful) { | ||
| val hobbyResponse = response.body() | ||
| _hobbyState.value = hobbyResponse?.result?.hobby |
kez-lab
reviewed
Nov 19, 2024
Comment on lines
+8
to
+20
| class TokenInterceptor : Interceptor { | ||
| override fun intercept(chain: Interceptor.Chain): Response { | ||
| val token = getAccessToken() | ||
| val requestBuilder = chain.request().newBuilder() | ||
|
|
||
| if (token != null) { | ||
| requestBuilder.addHeader("token", token) | ||
| Log.d("token", token) | ||
| } | ||
|
|
||
| return chain.proceed(requestBuilder.build()) | ||
| } | ||
| } |
There was a problem hiding this comment.
만약 토큰이 변경되었을 때는 어떻게 해야할까요? 외부에서 OkHttpClient 를 싱글 인스턴스로 만들어 활용중인데요, 로그아웃 기능이 있다면 어떻게 대응할지 고민해보면 좋을 것 같아요!
Comment on lines
+7
to
+12
| // TokenInterceptor 에서 Header intercept | ||
| interface MyService { | ||
| @GET("/user/my-hobby") | ||
| fun getMyHobby( | ||
| ): Call<ResponseMyHobbyDto> | ||
| } |
There was a problem hiding this comment.
결국 이 API의 path가 user인데 클라이언트단에서 My 라는 관심사로 분리하신 이유가 궁금합니다
Comment on lines
31
to
36
| @Composable | ||
| fun MyScreen(paddingValues: PaddingValues) { | ||
| val viewModel: MyViewModel = viewModel() | ||
| viewModel.getUserHobby() | ||
|
|
||
|
|
There was a problem hiding this comment.
paddingValues가 변할때마다 viewModel.getUserHobby()가 호출될 것 같아요!
Comment on lines
98
to
110
| MyMovieInfo( | ||
| viewModel = MyViewModel(), | ||
| viewModel = viewModel, | ||
| titleResId = R.string.my_watching, | ||
| imageResId = R.drawable.ic_warning, | ||
| noContentResId = R.string.my_no_watching | ||
| ) | ||
|
|
||
| MyMovieInfo( | ||
| viewModel = MyViewModel(), | ||
| viewModel = viewModel, | ||
| titleResId = R.string.my_wish, | ||
| imageResId = R.drawable.ic_warning, | ||
| noContentResId = R.string.my_no_wish | ||
| ) |
Comment on lines
45
to
+59
| // textStyle 변경을 위한 textFieldValue 추적 | ||
| val idState = remember { mutableStateOf(TextFieldValue()) } | ||
| val passwordState = remember { mutableStateOf(TextFieldValue()) } | ||
| val hobbyState = remember { mutableStateOf(TextFieldValue()) } | ||
| // id text remeber를 통한 변수 변경 | ||
| var textId by remember { mutableStateOf("") } | ||
| // password text remeber를 통한 변수 변경 | ||
| var textPasswd by remember { mutableStateOf("") } | ||
| // hobby text remember를 통한 변수 변경 | ||
| var textHobby by remember { mutableStateOf("") } | ||
| val lifecycleOwner = LocalLifecycleOwner.current | ||
| val authState by viewModel.authState.collectAsStateWithLifecycle(lifecycleOwner) | ||
| val context = LocalContext.current | ||
| // 모든 textFiled가 채워졌는지 판단하는 변수 | ||
| val allFieldFilled = idState.value.text.isNotEmpty() && passwordState.value.text.isNotEmpty() | ||
| val allFieldFilled = idState.value.text.isNotEmpty() && passwordState.value.text.isNotEmpty() && hobbyState.value.text.isNotEmpty() |
There was a problem hiding this comment.
매번 ViewModel로 해당 상태를 넘기는 것보다 이 상태들을 이제 ViewModel에서 관리해보는 것은 어떨가요?
There was a problem hiding this comment.
추가적으로 궁금한게 idState와 textId가 왜 분리되어있는건지도 궁금합니다.
There was a problem hiding this comment.
lifecycleOwner도 기본이 아마 LocalLifecycleOwner.current일건데 추가적으로 정의할 필요가 있을까요?
Comment on lines
+78
to
+84
| if(response.isSuccessful) { | ||
| val loginResponse = response.body() | ||
| loginResponse?.result?.token?.let { | ||
| // SharedPreference에 토큰 저장 | ||
| saveToken(it) | ||
| } | ||
| _authState.value = AuthState.Success(AuthType.LOGIN) |
There was a problem hiding this comment.
여기서 토큰이 없는데도 AuthState.Success가 호출될 수 있네요, 수정이 필요할 것 같습니다~!!
jihyunniiii
reviewed
Nov 20, 2024
| @Serializable | ||
| data class ResponseRegisterResultDto( | ||
| @SerialName("no") | ||
| val no: Int |
There was a problem hiding this comment.
직관적인 네이밍을 사용해도 좋을 것 같아요!
꼭 서버에서 내려주는 형태로 네이밍을 가져갈 필요는 없습니다.
Comment on lines
+7
to
+12
| // TokenInterceptor 에서 Header intercept | ||
| interface MyService { | ||
| @GET("/user/my-hobby") | ||
| fun getMyHobby( | ||
| ): Call<ResponseMyHobbyDto> | ||
| } |
There was a problem hiding this comment.
저두 궁금해요! 서비스를 분리하시는 기준이 있으신가요?
| fun saveUserId(userId: String) { | ||
| sharedPreferences.edit().putString("user_id", userId).apply() | ||
| fun getAccessToken(): String? { | ||
| Log.d("token", preferences.getString("token", null).toString()) |
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.
Related issue 🛠
Work Description ✏️
Screenshot 📸
https://rumbling-chip-236.notion.site/4-13fb4062d9c98071b814ced91871a8d3?pvs=4
Uncompleted Tasks 😅
To Reviewers 📢
서버 통신 코드를 작성하면서, 기존에 작성했던 비동기적 코드를 명확하게 이해하지 못하고 작성했다는 것을 깨달았던 과제였습니다.. 다음 Refactoring 간 레포지토리 구조로 나누어 clean Architecture로 구현하고 싶습니다. 아직 미숙한 것 같아, 피드백 마구 주시면 감사드립니다!