[feat] 네트워크 연결 상태 확인 (로딩,에러 뷰) #103
Conversation
l2zh
requested review from
Jokwanhee,
giovannijunseokim,
hyunjium and
gatttto
|
코드 쌉꿀마 🍠 수고하셨습니다 |
| val request = chain.request() | ||
| var response = chain.proceed(chain.request()) | ||
|
|
||
| when (response.code) { |
There was a problem hiding this comment.
현재는 if문을 써도 될 것 같은데 확장성 고려하신 건가요 ??
There was a problem hiding this comment.
넵! 에러코드가 아직 정리가 안된것 같아서 우선 500번대만 처리해 두었습니다!
|
|
||
| when (response.code) { | ||
|
|
||
| in SERVER_ERROR_START..SERVER_ERROR_END -> { |
There was a problem hiding this comment.
이 상수화가 필요한 상수화인지 생각해봐도 좋을 것 같아요! 물론 나쁘다는 의미는 아니지만, HTTP 코드 정도는 어느정도 상식으로 알고 계시는 분도 많다보니 오히려 어색한 느낌이 들기도 하네요 😅
There was a problem hiding this comment.
오...그런 부분은 생각 못해봤네요! 좋은 피드백 감사합니다!
| class NetworkErrorListenerImpl @Inject constructor() : NetworkErrorListener { | ||
| private var onApiCallFailedCallback: (() -> Unit)? = null | ||
|
|
||
| override fun onApiCallFailed() { | ||
| onApiCallFailedCallback?.invoke() | ||
| } | ||
|
|
||
| override fun setOnApiCallFailedCallback(callback: () -> Unit) { | ||
| onApiCallFailedCallback = callback | ||
| } | ||
|
|
||
| override fun clearOnApiCallFailedCallback() { | ||
| onApiCallFailedCallback = null | ||
| } | ||
| } |
There was a problem hiding this comment.
오 너무 좋은 설계인 것 같은데요 ?? 👍
| import android.net.NetworkCapabilities | ||
| import androidx.lifecycle.LiveData | ||
|
|
||
| class NetworkStateLiveData(context: Context) : LiveData<Boolean>() { |
There was a problem hiding this comment.
해당 클래스를 싱글톤으로 만드는건 어떨까요 ??
There was a problem hiding this comment.
해당 클래스는 이미 힐트 모듈로 정의해주어서 애플리케이션의 전역범위에서 동일한 인스턴스를 제공하는 것 같은데 혹시 잘못된 부분이 있을까요??
There was a problem hiding this comment.
아 그러네요 ㅋㅋㅋㅎ 확인했습니다 !
| override fun onViewCreated(view: View, savedInstanceState: Bundle?) { | ||
| super.onViewCreated(view, savedInstanceState) | ||
|
|
||
| backPressed() |
There was a problem hiding this comment.
조금 더 명확한 함수명을 사용해도 좋을 것 같아요!
| backPressed() | |
| overrideOnBackPressed() |
| @@ -28,7 +28,7 @@ class SplashFragment : BindingFragment<FragmentSplashBinding>(R.layout.fragment_ | |||
|
|
|||
| private fun showSplash() { | |||
| lifecycleScope.launch { | |||
| delay(2000) | |||
| delay(SPLASH_DISPLAY_LENGTH) | |||
There was a problem hiding this comment.
오 이건 너무 좋은 상수화네요! 상수화함으로서 이름이 생겨 delay의 의미까지 명확해진 좋은 코드인 것 같습니다 👍
| override fun clearOnTokenRefreshFailedCallback() { | ||
| onTokenRefreshFailedCallback = null |
| val networkStateLiveData: LiveData<Boolean> | ||
| val isLoading: LiveData<Boolean> |
There was a problem hiding this comment.
LiveData 를 사용하신 이유가 궁금합니다!!
There was a problem hiding this comment.
엇 기존에 베이스 뷰모델로 하려던게 인스턴스 문제 때문에 repository로 옴겼는데 그 과정에서 놓친 부분이네요ㅠㅠ
도메인 레이어와 데이터 레이어에서는 순수 코틀린 코드로만 이루어져야 안드로이드 프레임워크와 결합이 생기지 않는데 지금 코드는 아키텍쳐에 위배되는 코드인 것 같네요..!
짚어주셔서 감삼당~
| navController.popBackStack(R.id.loginFragment, false) | ||
| navController.navigate(R.id.loginFragment) | ||
| authTokenRefreshListener.setOnTokenRefreshFailedCallback { | ||
| CoroutineScope(Dispatchers.Main).launch { |
There was a problem hiding this comment.
코루틴 디스패처를 외부 주입으로 하시는 건 어떠신가요~?
📌 관련 이슈
📷 screenshot
default.mp4
📝 Work Desciption
📚 Reference 혹은 궁금한 사항들
평소에 안하던 방법으로 해서 문제가 있는것 같으면 바로 알려주세요...!