[FIX/#300,#302] Domain 레이어 Paging3 제거 & FlowWithLifecycle 적용 #304

Marchbreeze · 2023-10-15T18:41:19Z

closed [MOD] flowWithLifeCycle로 Flow 적용 #300
closed [FIX] Paging3 Domain 레이어 제외 #302

⛳️ Work Description

…into fix/#302-paging3-domain

kkk5474096

구현 잘 되는것도 확인했나여
flow는 livedata와 다르게 초기 값을 가지고 있어서
바로 해당 초기값을 화면에 진입하고 onStart 되자마자 collect하고 있어서
LiveData에서 의도한 동작시점과 다를수도 있어서 확인이 필요합니다

kkk5474096 · 2023-10-16T05:56:44Z

app/src/main/java/com/el/yello/presentation/pay/BillingManager.kt

@@ -33,23 +33,19 @@ class BillingManager(private val activity: Activity, private val callback: Billi
    val isPurchasing: StateFlow<Boolean> = _isPurchasing


여기 결제관련 BillingManager가 app에 presentation에 있어서 어색하게 느껴져서 질문한거 같은데
결제같은 경우 독립적으로 Module로(결제 모듈) 따로 만들어서 사용하면 분리가 더 잘 이루어질 것 같습니다.

flow 초기값들을 대부분 UiState.Empty로 설정하고 return으로 설정해둬서 괜찮을 것 같다만, 테스트 서버가 풀려야지 확인이 가능해서 ㅠㅠ 그때 다시 확인해보겠슴둥

결제 모듈 분리 ... 더 많이 공부해보고 출발해보겠습니닷

카카오SDK 서버통신이라 구글 Billing Client 같이 우리 서버 말고 외부 서버랑 통신하는 과정들은 따로 Repository와 DataSource로 안나누어주어도 괜찮나요?

카카오 로그인도 하나의 모듈로 만들어서 관리하면 편하긴 합니다.
외부 결제 구현의 경우 응답 객체 자체도 외부 라이브러리이기에 모듈화해서 뷰나 뷰모델에서 사용하고 있습니다.
물론 둘다 응답 값들을 우리 도메인형태로 매핑해서 사용하는 방법도 있습니다

kkk5474096

좋습니다~

b1urrrr

급삘받아서 잔뜩 써봤습니다...
플로우 진짜 쉽지 않네요! 코드 잘 보고 갑니다!

b1urrrr · 2023-10-18T05:12:41Z

app/src/main/java/com/el/yello/presentation/auth/SignInViewModel.kt

+    private val _getDeviceTokenError = MutableStateFlow(false)
+    val getDeviceTokenError: StateFlow<Boolean> = _getDeviceTokenError


Boolean 값을 저장하는 변수인데 조금 더 명확하게 네이밍해주는 건 어떨까요?!

와 이 링크 미쳐따

b1urrrr · 2023-10-18T05:28:25Z

app/src/main/java/com/el/yello/presentation/auth/SignInActivity.kt

@@ -78,21 +82,21 @@ class SignInActivity : BindingActivity<ActivitySignInBinding>(R.layout.activity_
                        // 403, 404 : 온보딩 뷰로 이동 위해 카카오 유저 정보 얻기
                        viewModel.getKakaoInfo()
                    } else {
-                        // 401 : 에러 발생
+                        // 나머지 : 에러 발생


83번째 줄에 viewModel.getKakaoInfo()를 호출하는 로직은 UI와 관련 없는 비즈니스 로직이라 뷰모델에서 처리하는 게 더 구조에 적합하지 않을까여?
뷰에서는 이런 로직이 보이지 않는 게 더 유지보수하기 좋을 것 같습니다!

b1urrrr · 2023-10-18T05:30:47Z

app/src/main/java/com/el/yello/presentation/auth/SignInActivity.kt

+    private var userName: String = String()
+    private var userGender: String = String()
+    private var userEmail: String = String()
+    private var userImage: String = String()


똑같이 빈 문자열로 초기화될 것 같은데 이렇게 바꾼 이유가 있나요?
별 이유 없다면 먼가 "" 요게 더 직관적으로 빈 문자열이라고 이해될 것 같은 늑김...

코드에서 ""처럼 초록색 스트링값 안봤으면 해서 추출하는 느낌으로 전환해봤서요..!

b1urrrr · 2023-10-18T05:33:01Z

app/src/main/java/com/el/yello/presentation/auth/SignInActivity.kt

+        viewModel.getDeviceTokenError.flowWithLifecycle(lifecycle).onEach { error ->
+            if (error) toast(getString(R.string.sign_in_error_connection))
+        }.launchIn(lifecycleScope)
    }

    // 카카오통 앱 로그인에 실패한 경우 웹 로그인 시도
    private fun observeAppLoginError() {
-        viewModel.isAppLoginAvailable.observe(this) { available ->
+        viewModel.isAppLoginAvailable.flowWithLifecycle(lifecycle).onEach { available ->
            if (!available) viewModel.startKakaoLogIn(this)
-        }
+        }.launchIn(lifecycleScope)


코드 스타일마다 다를 것 같긴 하지만... 지금 SignIn에 상태 관련된 변수가 많아서 sealed class 하나로 합쳐서 분기 처리하면 더 깔끔할 수도 있지 않을까 하는 생각이 들었습니다!

b1urrrr · 2023-10-18T05:40:45Z

app/src/main/java/com/el/yello/presentation/auth/SocialSyncActivity.kt

+        intent.apply {
+            val kakaoId = getLongExtra(EXTRA_KAKAO_ID, -1)
+            val email = getStringExtra(EXTRA_EMAIL)
+            val profileImage = getStringExtra(EXTRA_PROFILE_IMAGE)
+            val name = getStringExtra(EXTRA_NAME)
+            val gender = getStringExtra(EXTRA_GENDER)

-                Intent(this@SocialSyncActivity, OnBoardingActivity::class.java).apply {
-                    putExtra(EXTRA_KAKAO_ID, kakaoId)
-                    putExtra(EXTRA_EMAIL, email)
-                    putExtra(EXTRA_PROFILE_IMAGE, profileImage)
-                    putExtra(EXTRA_NAME, name)
-                    putExtra(EXTRA_GENDER, gender)
-                    addFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP)
-                    startActivity(this)
-                }
-                finish()
+            Intent(this@SocialSyncActivity, OnBoardingActivity::class.java).apply {
+                putExtra(EXTRA_KAKAO_ID, kakaoId)
+                putExtra(EXTRA_EMAIL, email)
+                putExtra(EXTRA_PROFILE_IMAGE, profileImage)
+                putExtra(EXTRA_NAME, name)
+                putExtra(EXTRA_GENDER, gender)
+                addFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP)
+                startActivity(this)


도메인 단에 데이터 클래스 하나 만들어서 parcelableExtra로 넘겨주면 좀 더 깔끔하지 않을까 싶은데 어떤가여 🧐

카카오 유저 도메인 만들어서 처리하면 좋을 것 같네요

근데 Parcelable은 android 라이브러리라서 domain에서 선언은 해주면 안되고 UiModel 새로 만들어서 파싱하거나 해야합니다

b1urrrr · 2023-10-18T05:57:48Z

app/src/main/java/com/el/yello/presentation/main/profile/info/ProfileFragment.kt

-                    is UiState.Failure -> {
-                        viewModel.isSubscribed = false
-                    }
+                    is UiState.Failure -> viewModel.isSubscribed = false


요것도 비즈니스 로직이 아닐까 싶어요!

b1urrrr · 2023-10-18T05:58:22Z

app/src/main/java/com/el/yello/presentation/main/profile/manage/ProfileManageActivity.kt

    }

    private fun initQuitBtnListener() {
        binding.btnProfileManageQuit.setOnSingleClickListener {
            AmplitudeUtils.trackEventWithProperties(
-                "click_profile_withdrawal",
-                JSONObject().put("withdrawal_button", "withdrawal1")
+                "click_profile_withdrawal", JSONObject().put("withdrawal_button", "withdrawal1")


시간 날 때 상수화도 ㅎ

b1urrrr · 2023-10-18T06:00:47Z

app/src/main/java/com/el/yello/presentation/main/recommend/RecommendFragment.kt

+    private fun initViewModelProvider() {
+        kakaoViewModel = ViewModelProvider(requireActivity())[RecommendKakaoViewModel::class.java]
+        schoolViewModel = ViewModelProvider(requireActivity())[RecommendSchoolViewModel::class.java]
+    }


by viewModels 안쓰고 Provider로 처리한 이유가 있나요?
위임으로 처리하면 훨씬 간결할 것 같아요!

뷰모델을 한 프래그먼트에서 두개를 받아올 때에는 Provider를 활용해서 받아와야했던 것 같은데, 저 로직 구현할 때 by viewModels 썼다가 안돼서 수정한 기억이 있습니닷

@Marchbreeze 오호 하나의 프래그먼트에서 여러 개의 뷰모델을 사용하면 안 된다고 생각했었는데 관심사 분리 측면에서는 오히려 효율적일 수 있겠네요!

찾아본 블로그 : https://velog.io/@cksgodl/android-%ED%95%98%EB%82%98%EC%9D%98-Framgent%EA%B0%80-%EC%97%AC%EB%9F%AC%EA%B0%9C%EC%9D%98-Viewmodel%EC%9D%84-%EC%82%AC%EC%9A%A9%ED%95%98%EB%8A%94-%ED%8C%A8%ED%84%B4%EC%9D%B4-%EC%9E%98%EB%AA%BB%EB%90%9C-%EA%B2%83%EC%9D%BC%EA%B9%8C

b1urrrr · 2023-10-18T06:05:11Z

app/src/main/java/com/el/yello/presentation/main/recommend/kakao/RecommendKakaoFragment.kt

+        with(binding) {
+            layoutRecommendFriendsList.isVisible = true
+            layoutRecommendNoFriendsList.isVisible = false
+            shimmerFriendList.startShimmer()
+            shimmerFriendList.isVisible = true
+            rvRecommendKakao.isVisible = false
+        }
    }

    private fun showFriendListScreen() {
-        binding.layoutRecommendFriendsList.isVisible = true
-        binding.layoutRecommendNoFriendsList.isVisible = false
-        binding.shimmerFriendList.stopShimmer()
-        binding.shimmerFriendList.visibility = View.GONE
-        binding.rvRecommendKakao.visibility = View.VISIBLE
+        with(binding) {
+            layoutRecommendFriendsList.isVisible = true
+            layoutRecommendNoFriendsList.isVisible = false
+            shimmerFriendList.startShimmer()
+            shimmerFriendList.isVisible = false
+            rvRecommendKakao.isVisible = true
+        }
    }

    private fun showNoFriendScreen() {
-        binding.layoutRecommendFriendsList.isVisible = false
-        binding.layoutRecommendNoFriendsList.isVisible = true
-        binding.shimmerFriendList.stopShimmer()
+        with(binding) {
+            layoutRecommendFriendsList.isVisible = false
+            layoutRecommendNoFriendsList.isVisible = true
+            shimmerFriendList.stopShimmer()
+        }


요런식으로 처리되는 부분이 많은 것 같은데 Boolean 인자를 받아서 값을 대입해주는 함수를 하나 만들면 조금 더 가독성이 개선될 것 같다는 생각이 들었습니다!

b1urrrr · 2023-10-18T06:10:11Z

app/src/main/java/com/el/yello/presentation/pay/PayActivity.kt

+                is UiState.Success -> {
+                    stopLoadingScreen()
+                    when (state.data?.productId) {
+                        YELLO_ONE -> {


여기 상수명도 뭐(열람권?)에 대한 상수값인지 정확하게 명시해주면 더 좋을 것 같아요!

…into fix/#302-paging3-domain

Marchbreeze added 10 commits October 15, 2023 18:51

[FIX/#302] 타임라인 뷰 Paging Repository 제거

da4b141

[FIX/#302] 프로필 flowWithLifecycle 적용

24dff9c

[FIX/#302] 추천친구 flowWithLifecycle 적용

c7934c8

[FIX/#302] 결제 flowWithLifecycle 적용

5bc8ef4

[FIX/#302] 타임라인 뷰 flowWithLifecycle 적용

eadfa38

[FIX/#302] 검색 뷰 flowWithLifecycle 적용

f499543

[FIX/#302] 온보딩 친구추가 뷰 flowWithLifecycle 적용

52ce691

[FIX/#302] 로그인 뷰 flowWithLifecycle 적용

6fe8431

[FIX/#302] 친구동의 뷰 flowWithLifecycle 적용

5d5fbe1

[FIX/#302] BillingManager 함수화 수정

742187a

Marchbreeze added FIX 🔨 버그 및 오류 해결 PULL REQUEST 🚀 pull reqeust 날리기 상호 🍀 Sangho's Task labels Oct 15, 2023

Marchbreeze self-assigned this Oct 15, 2023

Marchbreeze requested a review from a team as a code owner October 15, 2023 18:41

Merge branch 'develop' of https://github.com/team-yello/YELLO-Android …

3e5d302

…into fix/#302-paging3-domain

Marchbreeze changed the title ~~[FIX/#302,#300] Domain 레이어 Paging3 제거 & FlowWithLifecycle 적용~~ [FIX/#300,#302] Domain 레이어 Paging3 제거 & FlowWithLifecycle 적용 Oct 15, 2023

kkk5474096 reviewed Oct 16, 2023

View reviewed changes

Marchbreeze added 8 commits October 17, 2023 20:50

[FIX/#302] 로그인 뷰 flow 검증

e3a1e6b

[FIX/#302] 온보딩 친구추가 flow 검증

f650378

[FIX/#302] 프로필, 타임라인 flow 검증

3592911

[FIX/#302] 추천친구 flow 검증

07f5a0b

[FIX/#302] 검색 flow 검증

585b215

[FIX/#302] 결제 flow 검증

1d71ee8

[FIX/#302] 프로필 뷰 아이디 @ 고정 해제

7c2692e

[FIX/#302] 결제 flow 검증

d201cba

kkk5474096 approved these changes Oct 18, 2023

View reviewed changes

b1urrrr approved these changes Oct 18, 2023

View reviewed changes

Merge branch 'develop' of https://github.com/team-yello/YELLO-Android …

527b1b0

…into fix/#302-paging3-domain

Marchbreeze merged commit 1165ab7 into develop Oct 18, 2023
1 check passed

Marchbreeze mentioned this pull request Oct 18, 2023

[FIX] 비즈니스 로직 뷰모델 이동 #305

Closed

4 tasks

b1urrrr deleted the fix/#302-paging3-domain branch October 19, 2023 15:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FIX/#300,#302] Domain 레이어 Paging3 제거 & FlowWithLifecycle 적용 #304

[FIX/#300,#302] Domain 레이어 Paging3 제거 & FlowWithLifecycle 적용 #304

Marchbreeze commented Oct 15, 2023

kkk5474096 left a comment

kkk5474096 Oct 16, 2023

Marchbreeze Oct 16, 2023

Marchbreeze Oct 16, 2023

Marchbreeze Oct 16, 2023

kkk5474096 Oct 16, 2023

kkk5474096 left a comment

b1urrrr left a comment

b1urrrr Oct 18, 2023

Marchbreeze Oct 18, 2023

b1urrrr Oct 18, 2023

b1urrrr Oct 18, 2023

Marchbreeze Oct 18, 2023

b1urrrr Oct 18, 2023

b1urrrr Oct 18, 2023

kkk5474096 Oct 18, 2023

kkk5474096 Oct 18, 2023

b1urrrr Oct 18, 2023

b1urrrr Oct 18, 2023

b1urrrr Oct 18, 2023

b1urrrr Oct 18, 2023

Marchbreeze Oct 18, 2023

b1urrrr Oct 19, 2023

b1urrrr Oct 18, 2023

b1urrrr Oct 18, 2023

		@@ -33,23 +33,19 @@ class BillingManager(private val activity: Activity, private val callback: Billi
		val isPurchasing: StateFlow<Boolean> = _isPurchasing

		private val _getDeviceTokenError = MutableStateFlow(false)
		val getDeviceTokenError: StateFlow<Boolean> = _getDeviceTokenError

[FIX/#300,#302] Domain 레이어 Paging3 제거 & FlowWithLifecycle 적용 #304

[FIX/#300,#302] Domain 레이어 Paging3 제거 & FlowWithLifecycle 적용 #304

Conversation

Marchbreeze commented Oct 15, 2023

⛳️ Work Description

kkk5474096 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

kkk5474096 left a comment

Choose a reason for hiding this comment

b1urrrr 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