[Feat/#11] 7주차 과제 구현 #12

serioushyeon · 2024-12-09T17:08:57Z

Related issue 🛠

closed [FEAT] 7주차 과제 #11

Work Description ✏️

MVI로 리팩

Screenshot 📸

Uncompleted Tasks 😅

To Reviewers 📢

preference 의존성 주입을 수정하고 MVI구조로 리팩했습니다.
MVI가 처음이라 잘한건지 모르겠네요 ..ㅜ
이상한 부분은 피드백 주세요 다들 화이팅!!

boiledEgg-s

마지막 과제까지 너무 고생 많으셨습니다~!

boiledEgg-s · 2024-12-18T12:50:58Z

app/src/main/java/org/sopt/and/presentation/auth/signin/SignInScreen.kt

+                is SignInContract.SignInUiEffect.ShowSuccessSnackBar -> {
+                    CoroutineScope(Dispatchers.Main).launch {
+                        SnackBarUtils.showSnackBar(
+                            message = context.getString(R.string.sign_in_snackbar_login_success),
+                            actionLabel = context.getString(R.string.sign_in_snackbar_action_close)
+                        )
+                    }
+                    navigateToMy()


함수간 책임 분리가 필요해보입니다!!

스낵바를 보여주는 사이드이펙트에서 화면 전환 함수도 호출하는 것은 함수의 책임을 모호하게 한다고 생각해요.
두 개의 사이드이펙트로 분리하여 관리하는건 어떨까요??

boiledEgg-s · 2024-12-18T12:53:50Z

app/src/main/java/org/sopt/and/presentation/auth/signin/viewmodel/SignInViewModel.kt

+        }
+    }
+
+    fun signIn() {


접근제어자를 사용해서 UI단에서 불필요하게 호출하는 일이 없도록 관리해봅시다!

boiledEgg-s · 2024-12-18T12:59:23Z

app/src/main/java/org/sopt/and/presentation/auth/signin/viewmodel/SignInViewModel.kt

+                isLoading = true
+            )
+        )
+        viewModelScope.launch {


BaseViewModel에서 이벤트에 대한 코루틴 처리를 해주고 있는 것 같아요!
그렇다면 signIn 함수를 suspend 함수로 바꿔서 내부에 viewModelScope을 다시 사용하지 않도록 할 수 있을 것 같습니다!

boiledEgg-s · 2024-12-18T15:49:16Z

app/src/main/java/org/sopt/and/presentation/main/component/MainBottomNavigationBar.kt

-        screen.javaClass.canonicalName?.let { it1 -> popUpTo(it1) { inclusive = false } }
-        launchSingleTop = true
-    }
+            screen.javaClass.canonicalName?.let { it1 -> popUpTo(it1) { inclusive = false } }


it1같이 모호한 이름의 변수는 사용하지 않는게 좋을 것 같아요~

boiledEgg-s · 2024-12-18T15:49:47Z

app/src/main/java/org/sopt/and/presentation/mypage/viewmodel/MyViewModel.kt

+
+    private fun loadHobby() {
+        val token = preferenceUtil.getUserToken()
+        Log.d("my**", token.toString())


로그 발견!

boiledEgg-s · 2024-12-18T15:51:18Z

app/src/main/java/org/sopt/and/presentation/home/component/MainContentHorizontalPager.kt

+                mainContentState = mainContent[idx % mainContent.size],
+                onClick = onMainContentClicked,
+                totalPage = mainContent.size,
+                currentPage = idx % mainContent.size + 1


idx % mainContent.size를 변수로 선언해서 사용하면 더 일관적이고 가독성이 좋은 코드가 될 것 같네요!

kangyein9892

찰떡같이 잘하셨네요 ! 고생하셨어요 !

kangyein9892 · 2024-12-18T17:10:32Z

app/src/main/java/org/sopt/and/presentation/auth/signin/SignInScreen.kt

+                    CoroutineScope(Dispatchers.Main).launch {
+                        SnackBarUtils.showSnackBar(
+                            message = context.getString(R.string.sign_in_snackbar_login_success),
+                            actionLabel = context.getString(R.string.sign_in_snackbar_action_close)
+                        )
+                    }


여기서 CoroutineScope(Dispatchers.Main)을 사용한 이유가 있을까요?

kangyein9892 · 2024-12-18T17:10:51Z

app/src/main/java/org/sopt/and/presentation/auth/signin/SignInScreen.kt

-            } else {
-                viewModel.errorMessageState.value?.let { message ->
+
+                is SignInContract.SignInUiEffect.ShowErrorSnackBar -> {
                    CoroutineScope(Dispatchers.Main).launch {


요기도 궁금합니다!

chattymin

안녕하세요~ 놀러왔습니다 ㅎㅎ
저번에 코리 남겨달라고 해서 남기러 왔는데 너무 잘해서 딱히 남길게 없네요...
코리 보시며 생각해보시고 Reply남기고 저 테그해주셔도 좋고, 따로 연락 주셔도 좋으니 편하게 질문주세요 :)

앱잼 화이팅~ 🚀

chattymin · 2024-12-19T03:47:35Z

app/src/main/java/org/sopt/and/presentation/auth/signin/viewmodel/SignInViewModel.kt

+                is BaseResult.Success -> {
+                    updateState(
+                        currentState.copy(
+                            isLoading = false
+                        )
+                    )
+                    preferenceUtil.saveUserToken(result.data.token)
+                    postEffect(SignInUiEffect.ShowSuccessSnackBar)
+                    postEffect(SignInUiEffect.NavigateToMy)
+                }
+
+                is BaseResult.Error -> {
+                    updateState(
+                        currentState.copy(
+                            isLoading = false,
+                            errorMessage = result.message
+                        )
+                    )
+                    postEffect(SignInUiEffect.ShowErrorSnackBar(result.message))
+                }


이건 제 스타일이긴 한데 아래처럼 성공, 실패에 상관 없이 진행되는 로직의 경우 when문 밖에 빼두고 같이 쓰는 편이에요

updateState( currentState.copy( isLoading = false ) )

chattymin · 2024-12-19T03:54:05Z

app/src/main/java/org/sopt/and/presentation/auth/signup/viewmodel/SignUpViewModel.kt

+    private fun validateUserName(username: String) =
+        username.isNotBlank() && username.length <= 8
+
+    private fun validatePassword(password: String) =
+        password.isNotBlank() && password.length <= 8
+
+    private fun validateHobby(hobby: String) =
+        hobby.isNotBlank() && hobby.length <= 8


이런 숫자들은 상수로 빼주면 좋습니다.

앱잼을 하다보면 다른사람이 작성한 코드를 수정할 일도 생겨요. 그럴때 8 이라는 숫자로 있으면 이게 뭘 의미하는지 찾기 힘들 수 있어요.

chattymin · 2024-12-19T03:55:36Z

app/src/main/java/org/sopt/and/presentation/home/HomeContract.kt

+        val mainContents: List<HomeContent> = emptyList(),
+
+        val commonContents: List<HomeCommonContent> = emptyList(),


List의 경우 불변 객체가 아닌, 읽기전용 객체에요. 그렇기 때문에 리컴포지션의 대상이 됩니다.
그렇기 때문에 @stable, @Immutble과 같은 어노테이션을 사용하거나, ImmutableList를 사용한다면 리컴포지션을 줄일 수 있어요. 단, 어노테이션을 붙일때는 여러 조건이 존재하기 때문에 꼭 찾아보시고 사용하시는걸 추천드려요

chattymin · 2024-12-19T03:57:45Z

app/src/main/java/org/sopt/and/presentation/home/HomeContract.kt

+    }
+
+    sealed class HomeUiEffect : UiEffect
+}


진짜진짜 사소한거긴 한데 저 빨간색 동그라미 거슬리지 않나요?

EOL(End Of Line)을 나타내는 에러표시에요. 파일의 마지막에 공백 엔터를 한줄 넣어서 컴파일러에게 "이 파일에 작성된 코드가 끝났다" 라는걸 알려주는 역할을 해요.

안쓰면 문제가 있냐구요? 적어도 kotlin 안드로이드에선 없어요. C언어 컴파일러인 gcc에선 없으면 에러난다고 하더라구요.
저처럼 앞으로 코드 작성하실때 신경쓰였으면 하는 마음에 코리 달았어요 ㅋㅋ

chattymin · 2024-12-19T04:03:24Z

app/src/main/java/org/sopt/and/presentation/home/component/CommonContentHorizontalColumn.kt

            Text(
-                text = commonContentState.mainTitle,
+                text = commonContent.mainTitle,
                fontSize = 18.sp,
                color = White,
                fontWeight = FontWeight.Bold


text에 엄~청나게 긴 값을 한번 넣어보실래요?
그러면... 아이콘이 밀려서 사라질거에요

text자체에 weight를 넣어버리는 방법도 있고, horizontalArrangemnet를 사용하는 방법도 좋을 것 같습니다 :)

chattymin · 2024-12-19T04:05:53Z

app/src/main/java/org/sopt/and/presentation/main/MainScreen.kt

+    LaunchedEffect(Unit) {
+        viewModel.sendEvent(MainContract.MainUiEvent.LoadUserToken)
+    }


LaunchedEffect를 사용해서 viewModel에 있는 함수를 실행시키는 것과 viewModel에서 init을 사용해서 함수를 실행시키는 것. 어떤 차이가 있을까요? 두 차이점을 찾아보고 상황에 맞게 사용한다면 좋을 것 같아요

여기선 큰 차이 없을거 같긴 하네요...

chattymin · 2024-12-19T04:08:06Z

app/src/main/java/org/sopt/and/presentation/util/BaseViewModel.kt

+    private val _event: MutableSharedFlow<Event> = MutableSharedFlow()
+    val event = _event.asSharedFlow()
+
+    private val _effect: Channel<Effect> = Channel()
+    val effect = _effect.receiveAsFlow()


sharedFlow와 channel과 같은 두가지 방법을 사용하신 이유가 있나요??
차이점을 찾아보시고 명확한 이유가 있다면 좋을 것 같아요 :)

jm991014

다른 분들이 너무 잘 달아주셔서 더 달게 없네용..ㅎㅎ 고생하셨습니다!

jm991014 · 2024-12-19T13:01:10Z

app/src/main/java/org/sopt/and/presentation/auth/signup/SignUpScreen.kt

+                            SignUpContract.Field.UserName,
+                            isFocused


field = SignUpContract.Field.UserName, isFocused = isFocused

다른데는 다 되어 있는데 여긴 왜 없나요!?

jm991014 · 2024-12-19T13:06:29Z

app/src/main/java/org/sopt/and/presentation/auth/signup/SignUpScreen.kt

+                            SignUpContract.Field.Password,
+                            isFocused


여기두용

jihyunniiii

고생하셨습니다 ~

jihyunniiii · 2024-12-25T04:34:32Z

app/src/main/java/org/sopt/and/presentation/auth/signup/SignUpScreen.kt

+                is SignUpContract.SignUpUiEffect.ShowSuccessToast -> {
+                    context.showToast(context.getString(R.string.sign_up_toast_success))
+                    navigateToSignIn()
                }


요기도 함수의 책임이 분리되면 좋겠네요

jihyunniiii · 2024-12-25T04:36:04Z

app/src/main/java/org/sopt/and/presentation/auth/signup/viewmodel/SignUpViewModel.kt

+                        isValid = isValid &&
+                                currentState.isPasswordValid &&
+                                currentState.isHobbyValid


이 값을 event의 인자로 넘겨줘도 좋을 것 같아요! 여기서는 상태를 업데이트 하는 로직만 담당할 수 있게요!

jihyunniiii · 2024-12-25T04:37:11Z

app/src/main/java/org/sopt/and/presentation/home/viewmodel/HomeViewModel.kt

+    private val dummyHomeContentRepository: DummyHomeContentRepository
+) : BaseViewModel<HomeUiState, HomeUiEvent, HomeUiEffect>(HomeUiState()) {
+
+    override fun reduceState(event: HomeUiEvent) {


이 함수는 어떤 책임을 져야 할까요?

serioushyeon added 9 commits December 9, 2024 15:23

chore/#11 : 프로젝트 구조 변경

32cfffa

refactor/#11 : 회원가입 MVI로 리팩토링

cdd7e7c

refactor/#11 : 로그인 MVI로 리팩토링

f88f9a4

del/#11 : 불필요 임포트 제거

d0ca365

refactor/#11 : home MVI로 리팩토링

bc1663f

del/#11 : 불필요 임포트 제거

326a2ca

refactor/#11 : MyPage MVI로 리팩토링

01278a0

refactor/#11 : MyPage MVI로 리팩토링

018c112

refactor/#11 : PreferenceUtil 의존성 리팩

21c9181

serioushyeon added the enhancement New feature or request label Dec 9, 2024

serioushyeon requested review from jm991014, kangyein9892, boiledEgg-s and jihyunniiii December 9, 2024 17:08

serioushyeon self-assigned this Dec 9, 2024

chore/#11 : main 폴더링 수정

1ff37f9

boiledEgg-s approved these changes Dec 18, 2024

View reviewed changes

kangyein9892 reviewed Dec 18, 2024

View reviewed changes

chattymin reviewed Dec 19, 2024

View reviewed changes

jm991014 approved these changes Dec 19, 2024

View reviewed changes

jihyunniiii reviewed Dec 25, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feat/#11] 7주차 과제 구현 #12

[Feat/#11] 7주차 과제 구현 #12

serioushyeon commented Dec 9, 2024 •

edited

Loading

boiledEgg-s left a comment

boiledEgg-s Dec 18, 2024

boiledEgg-s Dec 18, 2024

boiledEgg-s Dec 18, 2024

boiledEgg-s Dec 18, 2024

boiledEgg-s Dec 18, 2024

boiledEgg-s Dec 18, 2024

kangyein9892 left a comment

kangyein9892 Dec 18, 2024

kangyein9892 Dec 18, 2024

chattymin left a comment •

edited

Loading

chattymin Dec 19, 2024

chattymin Dec 19, 2024

chattymin Dec 19, 2024

chattymin Dec 19, 2024

chattymin Dec 19, 2024

chattymin Dec 19, 2024

chattymin Dec 19, 2024

jm991014 left a comment

jm991014 Dec 19, 2024

jm991014 Dec 19, 2024

jihyunniiii left a comment

jihyunniiii Dec 25, 2024

jihyunniiii Dec 25, 2024

jihyunniiii Dec 25, 2024

		val mainContents: List<HomeContent> = emptyList(),

		val commonContents: List<HomeCommonContent> = emptyList(),

[Feat/#11] 7주차 과제 구현 #12

Are you sure you want to change the base?

[Feat/#11] 7주차 과제 구현 #12

Conversation

serioushyeon commented Dec 9, 2024 • edited Loading

Related issue 🛠

Work Description ✏️

Screenshot 📸

Uncompleted Tasks 😅

To Reviewers 📢

boiledEgg-s 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

kangyein9892 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chattymin left a comment • edited Loading

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

jm991014 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jihyunniiii 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

serioushyeon commented Dec 9, 2024 •

edited

Loading

chattymin left a comment •

edited

Loading