[FEAT] WEEK4 - xml 필수과제 #8
Conversation
[feat/#1] week1 - xml / essential(필수)
|
|
||
| @Serializable | ||
| data class ResponseUserInfoDto( | ||
| @SerialName("code") |
There was a problem hiding this comment.
다른 dto에서도 code와 message가 공통적으로 있어서 매번 추가해줘야하는 비효율성을 위해 BaseResponse라는 Wrapper class가 있습니다! 이걸 사용하는건 개인의 취향이기 때문에 참고만 해주세용
There was a problem hiding this comment.
이 부분에 대해서 어떤 장단점이 있을지 고민해보시면 좋을 것 같아요!! 물론 제가 꿀팁공유에 올렸었지만요 🤭
| import retrofit2.Callback | ||
| import retrofit2.Response | ||
|
|
||
| data class LoginState( |
There was a problem hiding this comment.
특별한 이유가 있지 않은 이상, 한 파일에 한 클래스로만 작성해주세요 !
| val message: String, | ||
| val memberId: String? = null | ||
| ) | ||
| class LoginViewModel : ViewModel() { |
There was a problem hiding this comment.
한 파일에 액티비티와 뷰모델이 있는데 두개 파일로 분리해주세요!
| ) | ||
| class LoginViewModel : ViewModel() { | ||
| private val authService by lazy { ServicePool.authService } | ||
| val liveData = MutableLiveData<LoginState>() |
There was a problem hiding this comment.
프로퍼티명은 최대한 그 용도에 맞게 네이밍 부탁드려요! ex) loginState
더불어, 해당 프로퍼티가 다른 클래스에서 접근은 하되, 수정되지 못하게끔 backing properties 해주세요
| val memberId = response.headers()["location"] | ||
|
|
||
| liveData.value = LoginState( | ||
| isSuccess = true, |
There was a problem hiding this comment.
LoginState가 서버통신 처리 결과에 대해서 데이터를 담아준 것 같은데, message와 memberId가 꼭 필요할까요? 또한 지금은 로직이 복잡한 앱이 아니기 때문에 단순한 서버통신만 있겠지만, 추후 좀 더 큰 프로젝트를 운영한다면 다양한 요인의 실패가 있을거에요! 그래서 불린값으로 서버통신 성공 여부를 다루는 것보다는 다양한 케이스에 맞게 상수로 관리하는게 좋을 것 같습니다 ~!
There was a problem hiding this comment.
동의합니다! LoginState가 서버통신 처리 결과를 담는 것이라면 memberId는 따로 관리해주는 게 좋을 것 같네요!
There was a problem hiding this comment.
enum class로 SUCCESS, NETWORK_ERROR 등을 추가해서 관리하면 좋을 것 같네요!
| val message: String, | ||
| ) | ||
|
|
||
| class SignUpViewModel : ViewModel() { |
There was a problem hiding this comment.
LoginAcitivty.kt에 달았던 모든 코멘트와 동일합니다
| override fun getItemCount(): Int { | ||
| return friendList.size + 1 // 내 프로필을 포함하기 위해 +1을 함 | ||
| } |
There was a problem hiding this comment.
| override fun getItemCount(): Int { | |
| return friendList.size + 1 // 내 프로필을 포함하기 위해 +1을 함 | |
| } | |
| override fun getItemCount(): Int = friendList.size + 1 |
단순한 값을 반환해줄 때는 return 생략 가능합니다!
| ), | ||
| Friend( | ||
| profileImage = R.drawable.ic_person_black_24, | ||
| name = "이정하", |
| friendListAdapter.setFriendList(mockFriendList) | ||
| } | ||
| override fun onDestroyView() { | ||
| super.onDestroyView() |
There was a problem hiding this comment.
메모리 누수 방지를 위해 friendListAdapter도 null 처리해주시는게 좋아욥
| android:layout_marginStart="70dp" | ||
| android:layout_marginEnd="70dp" | ||
| android:hint="ID를 입력하세요" | ||
| android:id="@+id/et_id" |
There was a problem hiding this comment.
xml id명은 사진처럼 네이밍해주시는게 좋습니다! ex) edt_signup_id
| Log.e( | ||
| "LoginError", | ||
| "HTTP ${response.code()}: ${response.errorBody()?.string()}" | ||
| ) |
| val memberId = response.headers()["location"] | ||
|
|
||
| liveData.value = LoginState( | ||
| isSuccess = true, |
There was a problem hiding this comment.
동의합니다! LoginState가 서버통신 처리 결과를 담는 것이라면 memberId는 따로 관리해주는 게 좋을 것 같네요!
| if (loginState.isSuccess) { | ||
| val intent = Intent(this@LoginActivity, MainActivity::class.java).apply { | ||
| loginState.memberId?.let { memberId -> | ||
| putExtra("memberId", memberId) | ||
| } | ||
| } | ||
| startActivity(intent) |
There was a problem hiding this comment.
| if (loginState.isSuccess) { | |
| val intent = Intent(this@LoginActivity, MainActivity::class.java).apply { | |
| loginState.memberId?.let { memberId -> | |
| putExtra("memberId", memberId) | |
| } | |
| } | |
| startActivity(intent) | |
| if (loginState.isSuccess) { | |
| Intent(this@LoginActivity, MainActivity::class.java).apply { | |
| loginState.memberId?.let { memberId -> | |
| putExtra("memberId", memberId) | |
| startActivity(this) | |
| } | |
| } |
로도 가능할 것 같네요!
|
|
||
| override fun onViewCreated(view: View, savedInstanceState: Bundle?) { | ||
| super.onViewCreated(view, savedInstanceState) | ||
|
|
||
| memberId = activity?.intent?.getStringExtra("memberId") ?: "0" | ||
|
|
||
| // 회원 정보를 가져오는 API 호출 | ||
| memberId?.let { memberId -> | ||
| ServicePool.authService.getUserInfo(memberId.toInt()).enqueue(object : Callback<ResponseUserInfoDto> { |
| app:layout_constraintStart_toStartOf="parent" | ||
| app:layout_constraintTop_toTopOf="parent" | ||
| /> |
There was a problem hiding this comment.
윈도우라면 Ctrl + Alt + L로 코드 최적화 적용해주세요!
|
|
||
| @POST("member/login") | ||
| fun login( | ||
| @Body request: RequestLoginDto | ||
| ): Call<ResponseLoginDto> |
There was a problem hiding this comment.
저는 함수명을 구체적으로 적는 걸 좋아해서 postLoginToServer()처럼 작성해주는 것도 좋을 것 같아요!
|
|
||
| memberId = activity?.intent?.getStringExtra("memberId") ?: "0" | ||
|
|
||
| // 회원 정보를 가져오는 API 호출 |
There was a problem hiding this comment.
UDA원칙에 따라 상태관리를 중앙화 시키는게 좋아보입니다ㅎ
상태 변경, 데이터 흐름을 ViewModel이 관리 -> Fragment는 관찰 + 반영하는 로직
| companion object { | ||
| fun newInstance(memberId: String?): MyPageFragment { | ||
| val fragment = MyPageFragment() | ||
| val args = Bundle().apply { | ||
| putString("memberId", memberId) | ||
| } | ||
| fragment.arguments = args | ||
| return fragment | ||
| } | ||
| } | ||
|
|
||
| override fun onCreateView( | ||
| inflater: LayoutInflater, | ||
| container: ViewGroup?, | ||
| savedInstanceState: Bundle? | ||
| ): View { | ||
| _binding = FragmentMypageBinding.inflate(inflater, container, false) | ||
| return binding.root | ||
| } | ||
|
|
||
| override fun onViewCreated(view: View, savedInstanceState: Bundle?) { | ||
| super.onViewCreated(view, savedInstanceState) | ||
|
|
||
| memberId = activity?.intent?.getStringExtra("memberId") ?: "0" |
There was a problem hiding this comment.
newInstance()에서 memberId를 fragment의 arguments에 저장하는 로직 좋습니다!
그런데 onViewCreated에서 memberId를 activity intent로 직접 가져오고 있는데, newInstance를 통해 설정된 arguments를 사용하지 않고, activity에서 직접 데이터를 가져오면 어떤 문제가 생길 수 있을까요??
| val memberId = response.headers()["location"] | ||
|
|
||
| liveData.value = LoginState( | ||
| isSuccess = true, |
There was a problem hiding this comment.
enum class로 SUCCESS, NETWORK_ERROR 등을 추가해서 관리하면 좋을 것 같네요!
| liveData.value = LoginState(isSuccess = false, message = message) | ||
| } | ||
| } | ||
| class LoginActivity : AppCompatActivity() { |
| val liveData = MutableLiveData<SignUpState>() | ||
|
|
||
| fun signUp(signUpRequest: RequestSignUpDto) { | ||
| authService.signUp(signUpRequest).enqueue(object : Callback<ResponseSignUpDto> { |
There was a problem hiding this comment.
viewmodelScope를 사용해서 코루틴을 사용하는 것도 좋을 것 같아요!
| binding = ActivityMainBinding.inflate(layoutInflater) | ||
| setContentView(binding.root) | ||
|
|
||
| val memberId = intent.getStringExtra("memberId") // null일 경우에는 0으로 처리 |
| viewBinding true | ||
| dataBinding true |
There was a problem hiding this comment.
뷰바인딩과 데이터바인딩의 차이를 아시나요?
혹시 데이터바인딩을 어디에 사용하셨나요?! 😁
| implementation 'androidx.activity:activity:1.8.0' | ||
|
|
||
| implementation 'com.squareup.retrofit2:retrofit:2.9.0' | ||
| implementation 'org.jetbrains.kotlinx:kotlinx-serialization-json:1.4.1' | ||
| implementation 'com.jakewharton.retrofit:retrofit2-kotlinx-serialization-converter:1.0.0' | ||
|
|
||
| // define a BOM and its version | ||
| implementation(platform("com.squareup.okhttp3:okhttp-bom:4.10.0")) | ||
|
|
||
| // define any required OkHttp artifacts without version | ||
| implementation("com.squareup.okhttp3:okhttp") | ||
| implementation("com.squareup.okhttp3:logging-interceptor") | ||
| implementation("androidx.navigation:navigation-fragment-ktx:2.7.7") | ||
| implementation("androidx.navigation:navigation-ui-ktx:2.7.7") |
| private val okHttpClient = OkHttpClient.Builder() | ||
| .addInterceptor(loggingInterceptor) // 로깅 인터셉터 추가 | ||
| .connectTimeout(30, TimeUnit.SECONDS) // 연결 타임아웃 | ||
| .readTimeout(30, TimeUnit.SECONDS) // 읽기 타임아웃 | ||
| .writeTimeout(30, TimeUnit.SECONDS) // 쓰기 타임아웃 | ||
| .build() |
|
|
||
| @Serializable | ||
| data class ResponseUserInfoDto( | ||
| @SerialName("code") |
There was a problem hiding this comment.
이 부분에 대해서 어떤 장단점이 있을지 고민해보시면 좋을 것 같아요!! 물론 제가 꿀팁공유에 올렸었지만요 🤭
| } | ||
| } | ||
|
|
||
| private fun initObserver() { |
There was a problem hiding this comment.
initObserver라는 함수 이름으로 이 역할을 충분히 설명 가능한가요?
|
|
||
| class SignUpViewModel : ViewModel() { | ||
| private val authService by lazy { ServicePool.authService } | ||
| val liveData = MutableLiveData<SignUpState>() |
There was a problem hiding this comment.
liveData 라는 변수명은 이 변수가 하는 일을 설명해주지 못할 것 같습니다!!
| private fun handleError(response: Response<ResponseSignUpDto>) { | ||
| val message = when (response.code()) { | ||
| 400 -> "잘못된 요청입니다. 입력 값을 확인하세요." | ||
| 409 -> "이미 등록된 사용자입니다." | ||
| else -> "회원가입 실패: ${response.message()}" | ||
| } | ||
| liveData.value = SignUpState(isSuccess = false, message = message) | ||
| } |
There was a problem hiding this comment.
이런 방식으로 핸들링하는 것보다 더 좋은 방법이 있을 것 같아요!!
여기서 다른 분들이 달아놓은 리뷰에서 찾아보시길...!
| description = "Last Chance - 소수빈" | ||
| ) | ||
|
|
||
| val mockFriendList = listOf<Friend>( |
| super.onDestroyView() | ||
| _binding = null |
There was a problem hiding this comment.
| super.onDestroyView() | |
| _binding = null | |
| _binding = null | |
| super.onDestroyView() |
이친구는 이렇게 사용하는게 더 안정적이랍니다?
📗 WORK DESCRIPTION
TODO
📗VIDEO (RUN)
4week-xml-essential.mp4
📗TO REVIEWER
우와
과제하다가
해가 떴어요 ..!