DoTheBestMayB / UpbitAPI

업비트 API를 활용한 어플
1 stars 0 forks source link

마켓별 시세 UI 구현 #35

Closed DoTheBestMayB closed 2 years ago

DoTheBestMayB commented 2 years ago

참고 자료

Layout 라운드에 선 표시하기 https://faith-developer.tistory.com/205 AssistedInject 사용하기 https://charlezz.medium.com/assistedinject란-무엇인가-35fbc90069a1, https://aroundck.tistory.com/7987 ViewPager2 구현하기 https://developer.android.com/training/animation/screen-slide-2, https://developer.android.com/guide/navigation/navigation-swipe-view-2 viewBinding은 destory될 때 null 처리를 해야한다 https://developer.android.com/topic/libraries/view-binding#fragments Fragment에서 context 쉽게 사용하게 설정하기 https://curryyou.tistory.com/386 Fragment의 Life cycle과 LifecycleOwner http://pluu.github.io/blog/android/2020/01/25/android-fragment-lifecycle/, https://gift123.tistory.com/57 RecyclerView Adapter에서 메모리를 수동으로 해제해야 한다면 onDetachedFromRecyclerView 함수를 사용하기 https://ppizil.tistory.com/38

DoTheBestMayB commented 2 years ago
  1. UpbitTickerDataForUI 변환 로직으로 인해 코드가 이전보다 복잡해진 것 같습니다. 개선해야 하는 부분이 있는지, 지금이 적당한지 궁금합니다.
  2. context, binding의 nullalbe 때문에 코드가 더 길어진 것 같습니다. 이것은 어쩔 수 없나요? Dependency Injection의 boilerplate code를 Dagger, Hilt로 없앤 것처럼, nullable로 인한 boilerplate code를 처리하고 싶어요. https://github.com/DoTheBestMayB/UpbitAPI/pull/35#discussion_r881631573
BeokBeok commented 2 years ago

UpbitTickerDataForUI 변환 로직으로 인해 코드가 이전보다 복잡해진 것 같습니다. 개선해야 하는 부분이 있는지, 지금이 적당한지 궁금합니다.

모든 코드가 그렇듯이, 늘 개선할 부분들은 많습니다. 이전보다 코드가 더 복잡해지긴 했지만, 현재로써는 최선인듯 싶습니다. 이 부분은 나중에 시간이 날 때 간소화해보려고 노력해보시는 것이 좋을 것 같고, 이 부분을 너무 물고 넘어지는 것은 시간낭비일듯 합니다.

context, binding의 nullalbe 때문에 코드가 더 길어진 것 같습니다. 이것은 어쩔 수 없나요? Dependency Injection의 boilerplate code를 Dagger, Hilt로 없앤 것처럼, nullable로 인한 boilerplate code를 처리하고 싶어요.

그래서 뷰바인딩을 선호하지 않습니다. 데이터바인딩을 사용하면 nullable 문제가 해결됩니다. 비록 뷰바인딩이 데이터바인딩보다 성능 및 메모리상으로 이점이 있다고는 하나, 약간 더 우위일뿐 크게 차이나지 않으며 오히려 보일러 플레이트가 양산되므로 대부분 데이터바인딩을 사용합니다. 성능보다는 코드의 간결함과 가독성이 가장 중요하며, 그 다음이 성능입니다. 그렇다고 성능이 중요하지 않다는 것은 아닙니다.

DoTheBestMayB commented 2 years ago

문제점: 모든 Fragment가 동일한 viewModel을 가졌을 때, viewPager를 넘겨도 해당하는 마켓의 값이 노출되지 않음 원인: viewModel의 getMarkets 함수가 Fragment가 생성되는 최초 시점에만 호출됨 해결 방법: Fragment의 onResume() 함수에서 getMarkets 함수를 호출하도록 수정 92fdee7

근본적 원인: 모든 Fragment가 같은 viewModel을 observe하고 있으므로, 모든 Fragment의 Adapter가 동일한 데이터를 가지게 된다. ViewPager에서 인접한 Fragment를 미리 생성하는데, 생성된 후 onResume()이 호출된다. 왜냐하면 화면을 옆으로 넘겼을 때 로딩되지 않고 바로 보이는 것이 ViewPager의 의도이기 때문이다. 해결 방법: Fragment의 lifecycle을 고려해서 위 문제를 해결하는 것보다, Fragment가 각자의 viewModel을 가지도록 수정하는 것이 코드도 짧고 로직이 간단하므로 각자 viewModel을 가지도록 수정 6dc531f

참고자료 https://stackoverflow.com/questions/10024739/how-to-determine-when-fragment-becomes-visible-in-viewpager

BeokBeok commented 2 years ago

문제점: 모든 Fragment가 동일한 viewModel을 가졌을 때, viewPager를 넘겨도 해당하는 마켓의 값이 노출되지 않음 원인: viewModel의 getMarkets 함수가 Fragment가 생성되는 최초 시점에만 호출됨 해결 방법: Fragment의 onResume() 함수에서 getMarkets 함수를 호출하도록 수정 92fdee7

근본적 원인: 모든 Fragment가 같은 viewModel을 observe하고 있으므로, 모든 Fragment의 Adapter가 동일한 데이터를 가지게 된다. ViewPager에서 인접한 Fragment를 미리 생성하는데, 생성된 후 onResume()이 호출된다. 왜냐하면 화면을 옆으로 넘겼을 때 로딩되지 않고 바로 보이는 것이 ViewPager의 의도이기 때문이다. 해결 방법: Fragment의 lifecycle을 고려해서 위 문제를 해결하는 것보다, Fragment가 각자의 viewModel을 가지도록 수정하는 것이 코드도 짧고 로직이 간단하므로 각자 viewModel을 가지도록 수정 6dc531f

참고자료 https://stackoverflow.com/questions/10024739/how-to-determine-when-fragment-becomes-visible-in-viewpager

대체적으로 ViewPager 구조인 경우에는, 프래그먼트마다 데이터가 다르기 떄문에 각자 뷰모델을 가지는 것이 좋습니다. 만약 탭 포지션과 같은 정보를 프래그먼트끼리 공유를 해야한다면 activityViewModel을 사용하면 됩니다. 액티비티나 프래그먼트나 뷰모델을 여러개 가질 수 있기 때문에, activityViewModel과 viewModel을 적재적소에 잘 활용하시면 됩니다.

BeokBeok commented 2 years ago

@DoTheBestMayB 참고로 데이터를 불러오는 로직은 onStart가 아닌 onCreate(프래그먼트라면 onViewCreated)에서 해주세요.

BeokBeok commented 2 years ago

@DoTheBestMayB

UpbitTickerDataForUI 변환 로직으로 인해 코드가 이전보다 복잡해진 것 같습니다. 개선해야 하는 부분이 있는지, 지금이 적당한지 궁금합니다.

시간이 되실 때 해당 Repository를 보면서 분석해보세요. 코드를 작성하는 능력도 중요하지만, 다른 사람들 코드를 분석하는 능력도 굉장히 중요해요. 오히려 현업에서는 코드를 작성하는 업무보다 다른 사람들이 작성해놓은 코드를 분석하는 업무를 더 많이 하게 될거에요. 좋은 코드라고 장담은 하지 못하지만, 숫자 형식을 어떤식으로 포메팅하는지 위주로 보시면 큰 도움이 될 거에요.