feat/#9: week4 xml 과제 구현 #10

youjin09222 · 2024-05-03T09:35:09Z

Related issue 🛠

closed [FEAT] week4_xml 과제 #9

Work Description ✏️

서버 통신 (request header에 memberId 삽입 → Intercepter 활용)
List Users API 연결 (Follower List 제작)
로그인 성공/실패 시 메시지 제공
회원가입 성공/실패 시 메시지 제공

Screenshot 📸

KakaoTalk_20240508_163749744.mp4

To Reviewers 📢

열심히 고쳐보겠습니다! 잘 부탁드려요!😊
꼼꼼히 리뷰 달아주셔서 너무 감사합니다!
열심히 반영해보겠습니다🔥🔥

Marchbreeze

안녕하세요! 4주차 과제 3조 코드리뷰로 참여하게 된 32기, 33기 안드로이드 수료한 김상호입니다 :)

코드리뷰에 담긴 질문들은 꼭 고쳐야한다고 말씀드리는게 아닙니다! 코드를 활용해보는 과정에서 생각을 담은 개발을 하기 위한 하나의 질문을 던져드린다고 생각해주세요 ㅎㅎ (고치세요!가 아니라… 이유가 궁금하거나 알고 썼나 궁금해서 질문드리는…것입니다)
과하다싶을 정도로 커밋 쪼개는거 저는 굉장히 좋아하는 습관입니다! ㅎㅎ좋네요
그리고 안드로이드 실기기 사용하시는 것도… 안드 개발자의 자질이 보이네요 ^__^

함수화도 그렇고, 변수명이나 스코프 함수도 그렇고, xml 코드도 너무 깔끔하고, 기본적인 습관이 너무너무너무너무 잘 잡혀계시는 것 같아요 ! 코드도 너무 깔끔합니다 너무 … 잘하시네요 ㅜ 응원하겠습니다 ㅎㅎㅎ

Marchbreeze · 2024-05-03T12:22:34Z

app/src/main/java/com/sopt/now/test/data/Friend.kt

+    companion object {
+        const val TYPE_USER = 0
+        const val TYPE_FRIEND = 1
+    }


Friend 클래스의 변수에서는 활용되지 않는 것으로 보이는데, 타입 상수를 Friend 객체에 선언하신 이유가 있을까요?

찾기 쉽게 Friend와 관련된 부분을 모두 한곳에 넣었습니다..!
말씀해 주신 대로 활용되지 않는 곳에 선언하면 더 복잡할 것 같네요!
해당 부분 수정해서 FriendAdapter로 위치 옮겼습니다!

Marchbreeze · 2024-05-03T12:25:12Z

app/src/main/java/com/sopt/now/test/data/UserPreference.kt

+            return if (userId != null && userName != null && userPhone != null) {
+                UserData(userId, userName, userPhone)
+            } else {
+                null
+            }


이러한 방식은 주어진 조건이 참이면 값을 반환하고, 거짓이면 null을 반환하는 takeIf 를 활용하면 더욱 깔끔하게 표현할 수 있습니다! 다만 조건이 여러개인 경우 완전히 동일하지는 않을 수도 있는데, takeIf에 대해서 한번 알아보세요 ㅎㅎ

takeIf 처음 알게 됐는데 확실히 코드가 간결해지네요!
알려주셔서 감사합니다!

Marchbreeze · 2024-05-03T12:27:42Z

app/src/main/java/com/sopt/now/test/data/dto/response/ResponseUserInfoDto.kt

+@Serializable
+data class UserInfo(
+    @SerialName("authenticationId")
+    val authenticationId: String,
+    @SerialName("nickname")
+    val nickname: String,
+    @SerialName("phone")
+    val phone: String
+)


ResponseUserInfoDto의 data에 해당되는 UserInfo의 경우,

data class ResponseUserInfoDto( ) { @Serializable data class UserInfo( ) }

처럼 종속되어 활용할 수 있습니다! 다만 UserInfo가 Dto와 독립적으로 사용되는 경우가 더 있다면 사용하지 않는 것이 좋겠죠? 취향 차이입니다 ㅎㅎ

오 한번 적용해보겠습니다!

Marchbreeze · 2024-05-03T12:29:27Z

app/src/main/java/com/sopt/now/test/friend/FriendAdapter.kt

+    // 첫 번째 아이템
+    private val FIRST_ITEM_POSITION = 0


요 친구는 대문자와 snake case로 작성된 것으로 보아 상수같아요. 다만 변수는 소문자와 camel case로 작성되는 네이밍임은 아실 것 같아요. 클래스 내에서 상수를 변수와 차별해서 작성해주시려면 어떤 방식으로 작성하는 것이 더 권장될까요?

companion object로 상수를 정의하는게 더 좋을 것 같네요!
해당 부분 반영하겠습니다!!

Marchbreeze · 2024-05-03T12:29:57Z

app/src/main/java/com/sopt/now/test/friend/FriendAdapter.kt

+                val binding = ItemFriendBinding.inflate(LayoutInflater.from(parent.context), parent, false)
+                FriendViewHolder(binding)
+            }
+            else -> throw IllegalArgumentException("Invalid view type")


여기서 던진 exception은 어디로 갈까요?

Marchbreeze · 2024-05-03T12:39:00Z

app/src/main/java/com/sopt/now/test/presentation/HomeFragment.kt

+    private fun setRecyclerView(){
+        val friendAdapter = FriendAdapter(viewModel.mockFriendList)
+        binding.rvFriends.run {
+            layoutManager = LinearLayoutManager(requireContext())


LinearLayoutManager 선언도 xml에서 진행할 수 있는 것 알고 계신가요?
(이 부분은 지극히 취향차이입니다)

어머 xml에서 설정해놓고 또 설정했네요..?
해당 부분 지웠습니다!

Marchbreeze · 2024-05-03T12:40:30Z

app/src/main/java/com/sopt/now/test/presentation/LoginActivity.kt

+        setContentView(binding.root)
+
+        userPreference = UserPreference(this)
+        ApiFactory.initializeUserPreference(userPreference)


이처럼 apiFactory를 활용해서 직접적으로 관여하는 것을 비즈니스 로직으로 파악됩니다! 이러한 경우는 UI를 담당하는 액티비티 대신 다른 곳에서 활용하면 더 이상적인 코드가 될 것 같아요!

Marchbreeze · 2024-05-03T12:42:30Z

app/src/main/java/com/sopt/now/test/presentation/LoginActivity.kt

+    private fun getLoginRequestDto(): RequestLoginDto {
+        val id = binding.etLoginId.text.toString()
+        val password = binding.etLoginPw.text.toString()
+        return RequestLoginDto(
+            authenticationId = id,
+            password = password
+        )
+    }


Suggested change

private fun getLoginRequestDto(): RequestLoginDto {

val id = binding.etLoginId.text.toString()

val password = binding.etLoginPw.text.toString()

return RequestLoginDto(

authenticationId = id,

password = password

)

}

viewModel.login(RequestLoginDto(

authenticationId = binding.etLoginId.text.toString(),

password = binding.etLoginPw.text.toString()

)

)

처럼 추가적인 함수화 대신 위의 리스너에서 바로 활용해도 깔끔할 것 같아요

넵! 수정했습니다!

Marchbreeze · 2024-05-03T12:44:20Z

app/src/main/java/com/sopt/now/test/presentation/MainActivity.kt

+                R.id.menu_home-> {
+                    replaceFragment(HomeFragment())
+                    true
+                }


화살표 사이에 띄어쓰기 하고싶어요 .. 죄송합니다 이런거로 맨날 와비 괴롭혀서 ...
코드정리를 습관화하면 좋아요 ! 저는 단축키를 ctrl+s로 해둬서 코드 한 줄 적을때마다 정리하는 강박증을 가지고 있습니다 ㅋㅋ

앜ㅋㅋㅋㅋ 바로 띄어쓰기 하러가겠습니다🤣

Marchbreeze · 2024-05-03T12:45:29Z

app/src/main/java/com/sopt/now/test/presentation/MyPageFragment.kt

+
+    private fun initObserver() {
+
+        viewModel.userInfo()


이처럼 화면이 켜졌을 때 뷰모델의 함수가 실행되는 것은, 뷰모델에서 init{} 으로도 진행시킬 수 있답니다 ! 이 프래그먼트에 뷰모델이 연결되는 19번째 줄이 실행될 때 같이 실행되도록 할 수 있어요

오 덕분에 하나 더 배우네요! 바로 적용했습니다!

boiledEgg-s

깔끔합니다~ OB님께서 말씀하신 것처럼 커밋을 쪼개는 습관 너무 좋은 것 같네요!!
저도 본받아야겠네요ㅎㅎ

boiledEgg-s · 2024-05-05T07:19:21Z

app/src/main/java/com/sopt/now/test/presentation/HomeViewModel.kt

Reqres에서 제공하는 팔로워 API도 사용해봐요~

해당 부분 적용했습니다! 깜빡했는데 알려주셔서 감사해요!

boiledEgg-s · 2024-05-05T07:33:10Z

app/src/main/java/com/sopt/now/test/presentation/SignUpViewModel.kt

+                    val userId = response.headers()["location"]
+                    liveData.value = BaseState(
+                        isSuccess = true,
+                        message = "회원가입 성공 유저의 ID는 $userId 입니다."


이러한 하드코딩을 어떻게 없앨지 고민해보면 좋을 것 같아요!!

변수를 포함하는 문장들도 리소스 형태로 저장할 수 있답니다~

오 바로 적용해보겠습니다!!

chattymin

이번주도 고생하셨습니다!
어려우셨을텐데 너무 잘하셨네요!!

chattymin · 2024-05-05T08:53:36Z

app/src/main/java/com/sopt/now/test/data/AuthService.kt

+    @POST("member/join")
+    fun signUp(
+        @Body request: RequestSignUpDto,
+    ): Call<ResponseAuthDto>
+
+    @POST("member/login")
+    fun login(
+        @Body request: RequestLoginDto,
+    ): Call<ResponseAuthDto>


함수 명에 해당 api를 호출하는 방식이 적혀있으면 추후 관리하기에 편해요~

signUp -> postSignUp

넵 수정하겠습니다~!

chattymin · 2024-05-05T08:56:28Z

app/src/main/java/com/sopt/now/test/presentation/LoginViewModel.kt

+            ) {
+                if (response.isSuccessful) {
+                    val data: ResponseAuthDto? = response.body()
+                    val userId = response.headers()["location"]


상수를 따로 빼서 관리해봅시다!!

chattymin · 2024-05-05T08:58:42Z

app/src/main/java/com/sopt/now/test/presentation/SignUpViewModel.kt

+                        isSuccess = true,
+                        message = "회원가입 성공 유저의 ID는 $userId 입니다."
+                    )
+                    Log.d("SignUp", "data: $data, userId: $userId")


로그를 통해서 상태를 중간중간 확인하는것을 저는 정말 좋아합니다!
하지만, 실제 서비스가 출시될 때 log가 남아있게 된다면 중요한 데이터가 노출될 수 있으니 그때는 꼭 지우길 바랄게요!

넵 알겠습니다!!

chattymin · 2024-05-05T09:03:44Z

app/src/main/java/com/sopt/now/test/presentation/LoginViewModel.kt

+    val liveData = MutableLiveData<BaseState>()
+    val userIdLiveData = MutableLiveData<String?>()


liveData 활용 너무너무 좋습니다!!
하지만 이렇게 사용하게 된다면 activity에서 해당 변수에 접근해버릴 수 도있습니다.
아래와 같은 방법을 참고해서 적용한다면 안정성이 높아집니다 :)

오 이 부분은 공부 해보고 적용해봐야겠네요! 알려주셔서 감사합니다!

hyeeum

너무 잘하시는 거 아닌지... 코드 리뷰에 지식을 공유하거나 질문을 하거나..등등 다양하게 적어보고 싶었는데 감탄만하다 가는 것 같네요..ㅎㅎ..이번주도 수고 많으셨습니닷! :)

hyeeum · 2024-05-05T14:06:07Z

app/src/main/java/com/sopt/now/test/data/ApiFactory.kt

+    class HeaderInterceptor : Interceptor {
+        @Throws(IOException::class)
+        override fun intercept(chain: Interceptor.Chain) : Response = with(chain) {
+            val newRequest = request().newBuilder()
+                .addHeader("memberId", userPreference.getUserId().toString())
+                .build()
+            proceed(newRequest)
+        }
+    }


저는 유진언니 코드보면서 인터셉터 이해했어요..이 자리를 빌어서 감사하다는 말씀을 드립니다...

hyeeum · 2024-05-05T14:13:14Z

app/src/main/java/com/sopt/now/test/data/BaseState.kt

+package com.sopt.now.test.data
+
+data class BaseState(
+    val isSuccess: Boolean,
+    val message: String
+)


오오 baseState 만드셨군요...! 확실히 다른 곳에서도 baseState를 사용하니까 깔끔하고 매번 작성할 필요가 없어서 좋은 것 같아요 저도 얼른 적용해보겠습니다..!

hyeeum · 2024-05-05T14:18:14Z

app/src/main/java/com/sopt/now/test/presentation/UserInfoViewModel.kt

+                    val error = response.errorBody()?.string()
+                    val gson = Gson()
+                    try {
+                        val errorResponse = gson.fromJson(error, ResponseAuthDto::class.java)
+                        liveData.value = BaseState(
+                            isSuccess = false,
+                            message = "회원 정보 조회 실패: ${errorResponse.message}" // 에러 메시지 사용
+                        )
+                    } catch (e: Exception) {
+                        liveData.value = BaseState(
+                            isSuccess = false,
+                            message = "회원 정보 조회 실패: 에러 메시지 파싱 실패"
+                        )
+                    }
+                }
+            }
+


오 저번에 동민오빠가 알려준 내용이군요! 저도 얼른 적용해봐야겠습니다..!

hyeeum · 2024-05-05T14:19:17Z

app/src/main/java/com/sopt/now/test/data/UserPreference.kt

+    // 사용자 아이디 저장
+    fun saveUserId(userId: String) {
+        with(sharedPreferences.edit()){
+            putString("userId", userId)
+            apply()
+        }
+    }
+
+    // 사용자 데이터 가져오기
+    fun getUserId(): String? {
+        with(sharedPreferences){
+            val userId = getString("userId", null)
+            return userId
+        }
+    }
+


와 스코프 함수 너무 잘 사용하시는데요? 저도 유진 언니 따라서 스코프를 잘 활용할 수 있도록 코드를 꼼꼼히 확인하는 능력을 길러보겠습니다..!

feat/#13: week6 xml 과제 구현

…youjin-park into feat/week4_xml

chattymin

정말 열심히 수정한게 보여서 감동입니다 흑흑
고생하셨어요~!!!!

youjin09222 added 18 commits May 3, 2024 15:31

move/#9: presentation 패키지로 이동

c254cc9

mod/#9: selfDescription에서 phone으로 변경

af3798c

chore/#9: 인터넷 권한 추가

7e4f1b9

chore/#9: retrofit2 라이브러리 추가

57a3172

chore/#9: BASE_URL 숨기기

5863d8b

chore/#9: Gson 추가

bd8a756

add/#9: RequestSignUpDto 추가

6ce5e5f

add/#9: RequestLoginDto 추가

04ce825

add/#9: ResponseAuthDto 추가

edcad07

feat/#9: signUp, login 인터페이스 추가

905ac81

feat/#9: ApiFactory 추가

02cbded

feat/#9: 회원가입 서버 통신 구현

d635e6f

feat/#9: 사용자 아이디 저장

956772d

feat/#9: 로그인 서버 통신 구현

5a2fd43

add/#9: ResponseUserInfoDto 추가

0a346e6

add/#9: Interceptor 추가

ad0d55f

feat/#9: 회원 정보 조회 서버 통신 구현

1e0e58d

feat/#9: 홈 화면에 회원 정보 적용

242824b

youjin09222 added the 📌 필수 label May 3, 2024

youjin09222 requested review from chattymin, kez-lab, boiledEgg-s and hyeeum May 3, 2024 09:35

youjin09222 self-assigned this May 3, 2024

Marchbreeze approved these changes May 3, 2024

View reviewed changes

youjin09222 changed the base branch from main-xml to develop-xml May 3, 2024 17:05

boiledEgg-s approved these changes May 5, 2024

View reviewed changes

chattymin requested changes May 5, 2024

View reviewed changes

hyeeum approved these changes May 5, 2024

View reviewed changes

feat/#9: coil 라이브러리 추가

461b231

youjin09222 and others added 14 commits May 25, 2024 05:16

add/#9: dataBinding 추가

b0c1f35

add/#9: timber 추가

2844a8d

feat/#9: Context에 showToast 추가

3d274e2

feat/#9: UiState 추가

4781a17

refactor/#9: sharedPreferences 수정

ae98201

refactor/#9: LoginLiveData Backing Property

ef411de

refactor/#9: 함수 및 변수명 변경 (LoginActivity)

2a37968

refactor/#9: postSignUp 코루틴으로 변경

d4cec71

refactor/#9: SignUpViewModel 코루틴으로 변경

8ca5997

refactor/#9: initPostSignUpObserver 수정

387f741

refactor/#9: getUserInfo 코루틴으로 변경

80ce0cb

refactor/#9: UserInfoViewModel 코루틴으로 변경

9873cc6

refactor/#9: initGetUserInfoObserver 수정

54bbdf7

Merge pull request #14 from NOW-SOPT-ANDROID/feat/week6_xml

2afe198

feat/#13: week6 xml 과제 구현

youjin09222 requested a review from chattymin June 6, 2024 19:23

youjin09222 added 14 commits June 7, 2024 07:40

chore/#9 : 패키지 구조 변경

8f97d65

add/#9 : showToast 확장 함수 추가

45e0c2e

refactor/#9 : AuthService에서 로그인 API 호출을 suspend 함수로 변경

a0154ee

refactor/#9 : LoginViewModel에 코루틴 및 UiState 적용

d7e4a53

refactor/#9 : LoginActivity에 UiState 적용

f70f2c5

mod/#9 : 로그 메시지 추출하여 적용

707250d

del/#9 : 필요없는 파일 삭제

4ea0fb5

refactor/#9 : getFriendInfo를 suspend 함수로 변경

9a0ead4

refactor/#9 : UserProfileViewModel로 통합

794ae60

rename/#9 : ProfileViewHolder 이름 변경

bc4e974

mod/#9 : 로그 메시지 추출하여 적용 (MyPageFragment)

758e5ab

refactor/#9 : HomeFragment에서 뷰모델 1개만 호출 및 UiState 적용

cf8a14b

Merge branch 'feat/week4_xml' of https://github.com/NOW-SOPT-ANDROID/…

6a84ee4

…youjin-park into feat/week4_xml

del/#9 : 필요없는 코드 삭제

8466a7b

chattymin approved these changes Jun 8, 2024

View reviewed changes

		val liveData = MutableLiveData<BaseState>()
		val userIdLiveData = MutableLiveData<String?>()

feat/#9: week4 xml 과제 구현 #10

Are you sure you want to change the base?

feat/#9: week4 xml 과제 구현 #10

Conversation

youjin09222 commented May 3, 2024 • edited Loading

Related issue 🛠

Work Description ✏️

Screenshot 📸

To Reviewers 📢

Marchbreeze left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

boiledEgg-s left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chattymin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hyeeum left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chattymin left a comment

Choose a reason for hiding this comment

youjin09222 commented May 3, 2024 •

edited

Loading