[7주차/필수] Repository 분리 - xml #17

Hyobeen-Park · 2024-06-07T14:40:24Z

Related issue 🛠

closed 7주차 필수 과제 - XML #15

Work Video ✏️

KakaoTalk_20240607_234435747.mp4

Marchbreeze

반갑습니다 효빈님, 과제 코드리뷰로 참여하게 된 32기, 33기 안드로이드 수료한 김상호입니다 :)

사실 다른 조 코드리뷰 담당이긴한데, 둘러보다가 효빈님 보고 반가워서 들렸어요!
혹시 저를... 기억하시려나요..^_^

코드 너무 깔끔하고, 아키텍처 분리까지 완전 깔끔하시네요 !!
코드리뷰에 담긴 질문들은 꼭 고쳐야한다고 말씀드리는게 아닙니다! 코드를 활용해보는 과정에서 생각을 담은 개발을 하기 위한 하나의 질문을 던져드린다고 생각해주세요 ㅎㅎ (고치세요!가 아니라… 이유가 궁금하거나 알고 썼나 궁금해서 질문드리는…것입니다)

실력은 충분해보이시니, 앞으로 앱잼도 꼭 좋은 팀 만나서 행복앱잼하시기를 응원하겠습니다 :)

Marchbreeze · 2024-06-07T19:45:48Z

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

+    private lateinit var userAdapter: UserAdapter
+    private lateinit var friendAdapter: FriendAdapter


Fragment의 경우에는 View와 객체의 생명주기가 달라서, Adapter 역시 binding과 같이 백패킹 설정 후 onDestroyView()에서 해제해주는 방식이 안전합니다!

Marchbreeze · 2024-06-07T19:46:44Z

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

-        initUserData()
-        getUserList()
-
+        homeViewModel.getUserInfo(activity?.intent?.getStringExtra(USERID) ?: "0")


activity? 보다는 requireActivity()가 더 안전할 것 같아요!
UserId를 스트링 값으로 저장하는 이유가 있나요?

Marchbreeze · 2024-06-07T19:48:32Z

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

-        getUserList()
-
+        homeViewModel.getUserInfo(activity?.intent?.getStringExtra(USERID) ?: "0")
+        homeViewModel.getFriendsInfo(2)


이렇게 onCreateView에서 뷰모델의 함수를 바로 실행시키는 경우는,
뷰모델에서 init으로 처리할 수 있습니다!
// 뷰모델

init { getFriendsInfo(2) } private fun getFriendsInfo( ... )

처럼요~

저도 이렇게 사용합니당

Marchbreeze · 2024-06-07T19:50:35Z

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

+                    showToastMessage(homeUserState.message)
+                }
+
+                else -> Unit


코틀린 문에서는 when() 에서 else 대신 모든 경우를 표시하는 것으로 권장되어 있습니다! 물론 저도 귀찮아서 가끔 잘 안지키지만...ㅎ

Marchbreeze · 2024-06-07T19:51:43Z

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

+    private fun showToastMessage(message: String?) {
+        Toast.makeText(context, message, Toast.LENGTH_SHORT).show()
    }


이런 친구들은 다른 뷰에서도 자주 활용되기 때문에 확장함수로서 따로 파일을 생성해두고, 공동으로 사용한다면 중복 코드를 줄일 수 있어요!

toast, snackbar 등등.. 자주 사용되는 것을 util로 빼서 관리하고 호출할 수 있어요

Marchbreeze · 2024-06-07T19:53:06Z

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

+        viewModelScope.launch {
+            _homeFriendState.value = UiState.Loading


사소한 차이고 스타일 차이긴 한데, 저는 viewModelScope 안에는 서버통신에 실제로 필요한 값들만 넣어두긴 합니다!

Suggested change

viewModelScope.launch {

_homeFriendState.value = UiState.Loading

_homeFriendState.value = UiState.Loading

viewModelScope.launch {

Marchbreeze · 2024-06-07T19:54:32Z

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

+        loginViewModel.loginState.flowWithLifecycle(lifecycle).onEach { loginState ->
+            when (loginState) {
+                is UiState.Success -> {
+                    showToastMessage("${loginState.data} 님 로그인에 성공했습니다")


이런 토스트 텍스트들도 모두 텍스트 추출해서 보관하는게 더 깔끔해질 것 같아요 ~

Marchbreeze · 2024-06-07T19:54:49Z

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

+                    Intent(this@LoginActivity, HomeActivity::class.java).apply {
+                        putExtra(USERID, loginState.data.toString())
+                        startActivity(this)


스코프 함수 활용 편안하네요 ㅠㅡㅠ

Eonji-sw

고생하셨어요!

Eonji-sw · 2024-06-09T14:25:53Z

app/src/main/java/com/sopt/now/data/datasourceimpl/AuthRemoteDataSourceImpl.kt

+        authService.getUserInfo(memberId = memberId)
+
+    companion object {
+        const val LOCATION = "location"


Eonji-sw · 2024-06-09T14:29:00Z

app/src/main/java/com/sopt/now/data/model/response/ResponseFriendsDto.kt

+@Serializable
+data class ResponseFriendsDto(
+    @SerialName("page")
+    val page: Int,


Suggested change

val page: Int,

val page: Int = -1,

기본값 설정해주면 더 좋을 것 같아요

Eonji-sw · 2024-06-09T14:30:02Z

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

 import retrofit2.Call
+import retrofit2.Response
 import retrofit2.http.Body
 import retrofit2.http.GET
 import retrofit2.http.Header
 import retrofit2.http.POST

 interface AuthService {
    @POST("member/join")


상수화해주면 좋을 것 같아요

Eonji-sw · 2024-06-09T14:30:36Z

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

        @Body request: RequestLoginDto,
-    ): Call<ResponseSignupDto>
+    ): Response<NullableBaseResponse<Unit>>


Response<NullableBaseResponse... 로 감싼 이유가 있을까요?

Eonji-sw · 2024-06-09T14:30:56Z

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

-        @Header("memberId") userId: String,
-    ): Call<ResponseUserInfoDto>
+    suspend fun getUserInfo(
+        @Header("memberId") memberId: String,


이 부분도 상수화 가능해요

Eonji-sw · 2024-06-09T14:40:00Z

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

+    private fun showToastMessage(message: String?) {
+        Toast.makeText(context, message, Toast.LENGTH_SHORT).show()
    }


toast, snackbar 등등.. 자주 사용되는 것을 util로 빼서 관리하고 호출할 수 있어요

Eonji-sw · 2024-06-09T14:42:00Z

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

+        with(binding) {
+            btnLoginSignin.setOnClickListener {
+                if (etLoginId.text.isEmpty()) {
+                    showToastMessage("아이디를 입력해주세요")


이런 것도 strings 파일로 빼면 좋을 것 같네요. 또한 해당 로직을 뷰모델로 빼는게 좋아보여요.

Eonji-sw · 2024-06-09T14:44:15Z

app/src/main/java/com/sopt/now/presentation/Mypage/MypageFragment.kt

+        }.launchIn(lifecycleScope)
+    }
+
+    private fun showToastMessage(message: String?) {


여러 곳에서 반복되어 사용되어지는걸 보니 재사용성을 높일 필요가 중요해보이네요!

Eonji-sw · 2024-06-09T14:44:51Z

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

+class LoginViewModel(
+    private val authRepository: AuthRepository,
+) : ViewModel() {
+    private val _loginState = MutableStateFlow<UiState<Int?>>(UiState.Empty)


SharedFlow와의 차이점이 뭘까요?

Eonji-sw · 2024-06-09T14:47:23Z

app/src/main/java/com/sopt/now/util/NullableBaseResponse.kt

BaseResponse와 분리하여 만든 이유가 있나요?

Hyobeen-Park added 13 commits May 25, 2024 00:10

[feat] #13 뷰모델 세팅 + UiState 추가

ff58fd7

[mod] #13 회원가입 뷰모델 분리

947c6ad

[chore] #13 패키지 수정

4930ee5

[feat] #13 로그인 dto 생성

25bdbd0

[mod] #13 로그인 뷰모델 분리

d52f3d4

[mod] #13 로그인 시 userId 전달

1bb35c2

[feat] #13 회원정보 dto 생성

c8853ca

[mod] #13 회원정보 조회 뷰모델 분리

f31e03f

[mod] #13 마이페이지 뷰모델 분리

2dafe7e

[feat] #13 친구 목록 조회 dto 생성

706e700

[mod] #13 친구 목록 조회 뷰모델 분리

a3a3f81

[chore] #13 코드 정리

3d9b2f8

[feat] #15 아이디 / 비밀번호 입력 없을 때 조건 추가

fb7fc91

Marchbreeze reviewed Jun 7, 2024

View reviewed changes

Eonji-sw reviewed Jun 9, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[7주차/필수] Repository 분리 - xml #17

[7주차/필수] Repository 분리 - xml #17

Hyobeen-Park commented Jun 7, 2024 •

edited

Loading

Marchbreeze left a comment

Marchbreeze Jun 7, 2024

Marchbreeze Jun 7, 2024

Marchbreeze Jun 7, 2024

Eonji-sw Jun 9, 2024

Marchbreeze Jun 7, 2024

Marchbreeze Jun 7, 2024

Eonji-sw Jun 9, 2024

Marchbreeze Jun 7, 2024

Marchbreeze Jun 7, 2024

Marchbreeze Jun 7, 2024

Eonji-sw left a comment

Eonji-sw Jun 9, 2024

Eonji-sw Jun 9, 2024

Eonji-sw Jun 9, 2024

Eonji-sw Jun 9, 2024

Eonji-sw Jun 9, 2024

Eonji-sw Jun 9, 2024

Eonji-sw Jun 9, 2024

Eonji-sw Jun 9, 2024

Eonji-sw Jun 9, 2024

Eonji-sw Jun 9, 2024

		private lateinit var userAdapter: UserAdapter
		private lateinit var friendAdapter: FriendAdapter

		viewModelScope.launch {
		_homeFriendState.value = UiState.Loading

[7주차/필수] Repository 분리 - xml #17

Are you sure you want to change the base?

[7주차/필수] Repository 분리 - xml #17

Conversation

Hyobeen-Park commented Jun 7, 2024 • edited Loading

Related issue 🛠

Work Video ✏️

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

Eonji-sw 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

Hyobeen-Park commented Jun 7, 2024 •

edited

Loading