[REFACTOR] 마이페이지 리팩토링 #357
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,6 @@ | ||
| package com.runnect.runnect.presentation.mypage | ||
|
|
||
| import android.app.Activity.RESULT_OK | ||
| import android.content.Intent | ||
| import android.os.Bundle | ||
| import android.view.View | ||
|
|
@@ -18,162 +17,196 @@ import com.runnect.runnect.R | |
| import com.runnect.runnect.binding.BindingFragment | ||
| import com.runnect.runnect.databinding.FragmentMyPageBinding | ||
| import com.runnect.runnect.presentation.MainActivity | ||
| import com.runnect.runnect.presentation.base.BaseViewModel | ||
| import com.runnect.runnect.presentation.login.LoginActivity | ||
| import com.runnect.runnect.presentation.mypage.dto.UserDto | ||
| import com.runnect.runnect.presentation.mypage.editname.MyPageEditNameActivity | ||
| import com.runnect.runnect.presentation.mypage.history.MyHistoryActivity | ||
| import com.runnect.runnect.presentation.mypage.reward.MyRewardActivity | ||
| import com.runnect.runnect.presentation.mypage.setting.MySettingFragment | ||
| import com.runnect.runnect.presentation.mypage.upload.MyUploadActivity | ||
| import com.runnect.runnect.util.analytics.Analytics | ||
| import com.runnect.runnect.util.analytics.EventName.EVENT_CLICK_GOAL_REWARD | ||
| import com.runnect.runnect.util.analytics.EventName.EVENT_CLICK_RUNNING_RECORD | ||
| import com.runnect.runnect.util.analytics.EventName.EVENT_CLICK_UPLOADED_COURSE | ||
| import com.runnect.runnect.util.extension.applyScreenEnterAnimation | ||
| import com.runnect.runnect.util.extension.getStampResId | ||
| import com.runnect.runnect.util.extension.repeatOnStarted | ||
| import com.runnect.runnect.util.extension.showSnackbar | ||
| import dagger.hilt.android.AndroidEntryPoint | ||
|
|
||
| @AndroidEntryPoint | ||
| class MyPageFragment : BindingFragment<FragmentMyPageBinding>(R.layout.fragment_my_page) { | ||
|
|
||
| private val viewModel: MyPageViewModel by activityViewModels() | ||
|
|
||
| private val userData: UserDto | ||
| get() = viewModel.userData.value | ||
|
|
||
| private val stampResId: Int | ||
| get() = activity?.run { | ||
| getStampResId( | ||
| stampId = userData.stampId, | ||
| resNameParam = RES_NAME, | ||
| resType = RES_STAMP_TYPE, | ||
| packageName = packageName | ||
| ) | ||
| } ?: 0 | ||
|
|
||
| private lateinit var resultEditNameLauncher: ActivityResultLauncher<Intent> | ||
| private var isVisitorMode: Boolean = MainActivity.isVisitorMode | ||
|
There was a problem hiding this comment. 방문자 모드 체크할 때 이런 식으로 companion object로 처리하는 게 최선인지 고민되더라구요,, There was a problem hiding this comment. @leeeha 방문자 모드인지를 판별하는 로직도 ViewModel로 들어가야하지 않을까 싶습니다. There was a problem hiding this comment. shared preference를 사용하는 것이 기존의 companion object를 사용하는 방법과 비교했을 때 어떤 장점이 있을까요? There was a problem hiding this comment. 기존 코드에서 고민이 되었던 건 '굳이 저 변수를 메인 액티비티의 전역 변수로 만들어야 하는가?' 였습니다. There was a problem hiding this comment. 현재 코드를 살펴보니 액세스 토큰 값을 기준으로 방문자 모드 여부를 체크하던데, 그 결과값을 메인 액티비티의 전역 변수가 아닌 shared preferences에 저장해두고 사용해도 편할 거 같다는 생각이 들었습니다~! |
||
|
|
||
| override fun onViewCreated(view: View, savedInstanceState: Bundle?) { | ||
| super.onViewCreated(view, savedInstanceState) | ||
|
|
||
| if (isVisitorMode) { | ||
| activateVisitorMode() | ||
| } else { | ||
| deactivateVisitorMode() | ||
| } | ||
| } | ||
|
|
||
| private fun activateVisitorMode() { | ||
| with(binding) { | ||
| setVisitorMode(true) | ||
|
|
||
| btnVisitorMode.setOnClickListener { | ||
| activity?.let { | ||
| Intent(it, LoginActivity::class.java).let(::startActivity) | ||
| it.finish() | ||
| } | ||
|
There was a problem hiding this comment. reflection을 평소에 자주 사용하시는 거 같은데 특별한 이유가 있는지 궁금합니다! There was a problem hiding this comment. @leeeha There was a problem hiding this comment. 궁금해서 GPT한테 물어봤어요 ㅎㅎ
|
||
| } | ||
| } | ||
| } | ||
|
|
||
| private fun deactivateVisitorMode() { | ||
| setVisitorMode(false) | ||
| addListener() | ||
| addObserver() | ||
| setResultEditNameLauncher() | ||
|
|
||
| viewModel.getUserInfo() | ||
|
There was a problem hiding this comment. 개인적인 생각인데 이 부분 각각 별도의 함수로 분리해서 deactivateVisitorMode() 함수 내부의 추상화 수준을 다른 것들과 맞춰주는 건 어떨까요? There was a problem hiding this comment. @unam98 함수 내부의 추상화 수준을 맞춘다는게 어떤 뜻인지 설명해줄 수 있을까요? There was a problem hiding this comment. @unam98 추가로 어차피 Compose로 리팩토링할 화면이기 때문에 제거될 코드 (dataBinding 등)는 그냥 유지했습니다~ There was a problem hiding this comment. 예를 들어서 setVisitorMode()부터 setResultEditNameLauncher()까지는 내부 로직이 드러나지 않게 함수로 감싸져있지만 아래부터는 내부 로직이 그대로 드러나는 모양이라 아랫부분들도 위처럼 맞춰주자는 것이었습니다. 추상화 수준이라는 표현보다는 캡슐화 수준이라는 표현이 좀 더 와닿을 수도 있겠네요. 클린코드라는 책에서 봤던 내용인데 이 책이 항상 정답은 아니겠지만 함수 분리를 통한 장점도 누리면서 좀 더 깔끔한 것 같아 개인적인 의견 내봤습니당. |
||
| with(binding) { | ||
| vm = viewModel | ||
| lifecycleOwner = [email protected] | ||
| } | ||
| } | ||
|
|
||
| private fun setResultEditNameLauncher() { | ||
| resultEditNameLauncher = | ||
| registerForActivityResult(ActivityResultContracts.StartActivityForResult()) { result -> | ||
| if (result.resultCode == RESULT_OK) { | ||
| val name = result.data?.getStringExtra(EXTRA_NICK_NAME) ?: "" | ||
| viewModel.updateUser( | ||
| userData.copy(nickName = name) | ||
|
There was a problem hiding this comment. var을 사용하지 않을 수 있는 방법이어서 좋은 거 같습니다! There was a problem hiding this comment. @leeeha |
||
| ) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private fun addListener() { | ||
| binding.ivMyPageEditFrame.setOnClickListener { | ||
| Intent(requireContext(), MyPageEditNameActivity::class.java).apply { | ||
| putExtra(EXTRA_NICK_NAME, userData.nickName) | ||
| putExtra(EXTRA_PROFILE, stampResId) | ||
| }.let(resultEditNameLauncher::launch) | ||
| } | ||
|
|
||
| binding.viewMyPageMainRewardFrame.setOnClickListener { | ||
| Analytics.logClickedItemEvent(EVENT_CLICK_GOAL_REWARD) | ||
| startActivityWithAnimation(MyRewardActivity::class.java) | ||
| } | ||
|
|
||
| binding.viewMyPageMainHistoryFrame.setOnClickListener { | ||
| Analytics.logClickedItemEvent(EVENT_CLICK_RUNNING_RECORD) | ||
| startActivityWithAnimation(MyHistoryActivity::class.java) | ||
| } | ||
|
|
||
| binding.viewMyPageMainUploadFrame.setOnClickListener { | ||
| Analytics.logClickedItemEvent(EVENT_CLICK_UPLOADED_COURSE) | ||
| startActivityWithAnimation(MyUploadActivity::class.java) | ||
| } | ||
|
|
||
| binding.viewMyPageMainSettingFrame.setOnClickListener { | ||
| moveToSettingFragment() | ||
| } | ||
|
|
||
| binding.viewMyPageMainKakaoChannelInquiryFrame.setOnClickListener { | ||
| inquiryKakao() | ||
| } | ||
| } | ||
|
|
||
| private fun moveToSettingFragment() { | ||
| val bundle = Bundle().apply { | ||
| putString(ACCOUNT_INFO_TAG, userData.email) | ||
| } | ||
|
|
||
| requireActivity().supportFragmentManager.commit { | ||
| this.setCustomAnimations(R.anim.slide_in_right, R.anim.slide_out_left) | ||
| replace<MySettingFragment>(R.id.fl_main, args = bundle) | ||
| } | ||
| } | ||
|
|
||
| private fun addObserver() { | ||
| repeatOnStarted( | ||
| { | ||
| viewModel.userState.collect { | ||
| when (it) { | ||
| is MyPageViewModel.UserState.Loading -> handleLoading(true) | ||
| is MyPageViewModel.UserState.UserUpdated -> handleSuccess() | ||
| is MyPageViewModel.UserState.Failure -> handleFailure() | ||
| } | ||
| } | ||
| }, | ||
| { | ||
| viewModel.eventState.collect { | ||
| when (it) { | ||
| is BaseViewModel.EventState.ShowSnackBar -> { | ||
| context?.showSnackbar(binding.root, it.message) | ||
| } | ||
|
There was a problem hiding this comment. 스낵바가 반복적으로 뜨는 문제.. 위니에서도 발생했었는데 덕분에 해결 방법 알 수 있었네요! 저도 얼른 적용해봐야겠어요! |
||
| is BaseViewModel.EventState.NetworkError -> { | ||
| context?.showSnackbar(binding.root, it.message) | ||
| } | ||
| else -> Unit | ||
| } | ||
| } | ||
| } | ||
| ) | ||
| } | ||
|
|
||
| private fun handleLoading(isLoading: Boolean) { | ||
| with(binding) { | ||
| indeterminateBar.isVisible = isLoading | ||
| ivMyPageEditFrame.isClickable = !isLoading | ||
| viewMyPageMainSettingFrame.isClickable = !isLoading | ||
| } | ||
| } | ||
|
|
||
| private fun handleSuccess() { | ||
| handleLoading(false) | ||
| viewModel.updateUser( | ||
| userData.copy(profileImgResId = stampResId) | ||
| ) | ||
| binding.tvMyPageUserName.text = userData.nickName | ||
| } | ||
|
|
||
| private fun handleFailure() { | ||
| binding.indeterminateBar.isVisible = false | ||
| } | ||
|
|
||
| private fun setVisitorMode(isVisitor: Boolean) { | ||
| with(binding) { | ||
| ivVisitorMode.isVisible = isVisitor | ||
| tvVisitorMode.isVisible = isVisitor | ||
| btnVisitorMode.isVisible = isVisitor | ||
| constraintInside.isVisible = !isVisitor | ||
| } | ||
| } | ||
|
|
||
| private fun startActivityWithAnimation(activityClass: Class<*>) { | ||
| activity?.let { | ||
| Intent(it, activityClass).let(::startActivity) | ||
| it.applyScreenEnterAnimation() | ||
| } | ||
|
There was a problem hiding this comment. ActivityExt.kt 파일에 확장함수로 만들어둬도 좋을 거 같습니다! |
||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
매~~우 사소하고 주관적인 의견이지만! EventState라는 네이밍이 약간 고민되네요!
보통 SharedFlow는 여러 이벤트를 처리할 때, StateFlow는 수신자를 최신 상태로 갱신할 때 사용하는 것으로 알고 있습니다. 그래서 EventState보다는 Event 또는 EventBus 같은 네이밍은 어떨까라는 생각이 들었어요.
EventState라고 하면 이게 이벤트인지, 상태인지 아주 약간 헷갈릴 수도 있을 거 같아서요 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
참고로 SharedFlow 내부 코드에서도 아래처럼 EventBus 라는 클래스가 있더라구요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@leeeha 오호 좋은 것 같습니다
UI State와 1회성 이벤트를 구분하기 위해 SharedFlow를 추가했으니 네이밍도 확실히 구분되면 좋을 것 같네요
EventBus는 파이프라인이라는 인식이 좀 강한 것 같아서 Event 케이스를 나누는 목적과는 쪼금 차이가 있는 것 같긴하네요
한 번 고민해보고 수정해볼게요 좋은 네이밍 생각나면 말씀해주세요~