[Refactor] Week7 필수 과제 #13
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yskim6772
reviewed
Dec 19, 2024
Member
yskim6772
left a comment
yskim6772
left a comment
There was a problem hiding this comment.
고생하셨습니다 !
조금 더 MVI 패턴에 가까운 코드를 작성하기 위해,
SideEffect와 Event를 고려하여, 리팩토링 해보면 좋을 것 같아요 !
Comment on lines
+8
to
+31
| fun handleErrorToast( | ||
| exception: Throwable?, | ||
| is400Error: Int, | ||
| context: Context, | ||
| is403Error: Int = R.string.fail_to_network, | ||
| is409Error: Int = R.string.fail_to_network, | ||
| ) { | ||
| return when (exception) { | ||
| is HttpException -> when (exception.code()) { | ||
| 400 -> context.toast(context.getString(is400Error)) | ||
| 403 -> context.toast(context.getString(is403Error)) | ||
| 409 -> context.toast(context.getString(is409Error)) | ||
| else -> context.toast(context.getString(R.string.fail_to_network)) | ||
| } | ||
|
|
||
| is IOException -> { | ||
| context.toast(context.getString(R.string.fail_to_network)) | ||
| } | ||
|
|
||
| else -> { | ||
| context.toast(context.getString(R.string.fail_to_login)) | ||
| } | ||
| } | ||
| } No newline at end of file |
Member
There was a problem hiding this comment.
오옹 따로 handleErrorToast 함수 만들어놓은 것 좋네요 ! 혹시 이 에러 핸들링이 login 서버통신에만 적용되는 거라면, 어디서 사용되는건지 명시해주면 좋을 것 같아요 !
Comment on lines
+5
to
+11
| fun NavController.navigateToSignUp() { | ||
| navigate(Routes.SignUp.screen) | ||
| } | ||
|
|
||
| fun NavController.navigateToLogin() { | ||
| navigate(Routes.Login.screen) | ||
| } No newline at end of file |
Member
There was a problem hiding this comment.
위 두 navigator를 같이 묶어두신 이유가 있을까요?!
| editorDummy: List<TodayTopData>, | ||
| top20Dummy: List<TodayTopData>, | ||
| scrollState: ScrollState = rememberScrollState(), | ||
| pagerState: PagerState = rememberPagerState(pageCount = { 6 }) |
jihyunniiii
reviewed
Dec 25, 2024
| @@ -0,0 +1,24 @@ | |||
| package org.sopt.and.domain.type | |||
|
|
|||
| enum class HomeTabType( | |||
There was a problem hiding this comment.
요 부분은 mvi가 아직 적용되지 않은 걸로 보이는데 맞나요?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related issue 🛠
Work Description ✏️
Screenshot 📸
저번과 동일합니다.
Uncompleted Tasks 😅
To Reviewers 📢
mvi 패턴을 깔끔하게 적용하지 못한 것 같아요... 따끔한 피드백 부탁드립니다!