[Feat] 7주차 필수과제

[SYAAINN]

Related issue 🛠

• closed [Feat] 7주차 필수과제 #12

Work Description ✏️

• MVVM -> MVI 구조로 마이그레이션

Screenshot 📸

• N/A

Uncompleted Tasks 😅

• Main 부분 MVI 구조 적용

To Reviewers 📢

생각보다 난이도가 높네요 ㅠㅠ.. 일단 Auth 부분만 MVI로 고쳐봤는데 어떤지 한번 리뷰를 받아보고 나머지도 바꿔보려고 합니다.

1. 텍스트 필드에 입력을 받는 것 또한 Event(Intent)로 간주한다고 봐서 그렇게 구성했는데 과연 지금처럼 받는 구조가 바람직한지 봐주시면 감사하겠습니다.
2. 저번에 생성했던 AuthState를 아직은 남겨놓았는데요. enum class LoadState (Idle, Loading, Success, Failure 보유)를 만들고 현재 Success 가 보유하는 값들을 상태로 다 빼는 것이 좋을지, 현재처럼 유지해도 괜찮을 지가 궁금합니다! 각각에는 어떤 이점이 있을까요?
3. 지금은 SignIn, SignUp 두개의 스크린을 ViewModel 부터 Contract 등을 Auth라는 이름으로 묶어서 진행하고 있는데요, 스크린 당 1개로 분리하는 것이 바람직할까요? 취향일까요? (프로젝트 볼륨이 커지거나 소셜로그인 등이 들어가게 되면 아마 분리해야할 것 같긴합니다 ,,,)

[hwidung]

수고하셨습니다! 코드 너무 깔끔해서 보기 편해요

[hwidung]

저는 State, Event, SideEffect, 뿐만 아니라 SignIn, SignUp까지 다 분리하는 것이 좋다(?) 라고 생각하고 구현을 했었는데, 재원이 오빠도 그렇고 민재오빠도 이렇게 구현한 걸 보고 더 가독성이 좋고, 한눈에 보기 편하다는 생각이 들었어요! 주요 기능에 따라 이렇게 묶어둔 것 보고 리팩토링 때 이렇게 적용해보고자 해요 ㅎ 감삼둥

[hwidung]

오.. 이렇게 반복되는 뷰모델을 재사용하기 위해 abstract class를 사용하셨네요..! BaseViewModel<AuthContract.AuthUiState, AuthContract.AuthSideEffect, AuthContract.AuthEvent>() 이런식으로 재사용하심으로써 가독성이 좋아진 것 같아요 !! 저도 리팩 때 적용해보겠습니답ㅎㅎㅎ

[cacaocoffee]

감격스럽네요 ㅠㅜㅠㅜ 완전 고수가 다됐어요!

[cacaocoffee]

바쁜 학기중에 이만큼이나 열심히 잘해왔다니 제가 다 뿌듯하고 행복해지네요
저의 생각을 남겨 놓으면서 앱잼도 화이팅 하시길 바래요

3번
MVI 패턴의 사용 이유는 compose 를 활용했을 때 UI 로직과 비즈니스 로직을 분리하기 위함이라고 생각해요

그러한 관점에서는 추후 유지보수를 위해선 SignIn, SignUp 두개의 스크린을 ViewModel 부터 Contract 등을 Auth라는 이름으로 묶어서 진행 하기 보단 각 스크린 별 필요한 state, 이벤트, sideeffect 분리하는 게 좀 더 좋아보이네요! 근거는 아래쪽에 가볍게 다뤄 볼테니 한번 참고해보세요!

객체지향의 5원칙 SOLID 엔 다음과 같은 원칙이 있어요

Single Responsibility Principle / 단일 책임 원칙
클래스를 다양한 관점에서 기능을 모으면 안 되며,
클래스는 상호작용을 하는 대상에 따라 분류해서 구성해야 한다 라는 의미를 가지는데요

작은 프로젝트에선 상관없으나 프로젝트의 규모가 커지구 나중에 로그인 쪽 수정과, 회원가입 쪽의 수정이 동시에 필요한 상황일 경우 일이 복잡해질수도 있다. 이런 단점을 가지게 되어서 분리하는 게 좋다는 의미예요

[cacaocoffee]

감격스럽네요 ㅠㅜㅠㅜ 완전 고수가 다됐어요!

[cacaocoffee]

스트링 값 분리하면 더 좋읗 거 같아요

[cacaocoffee]

1번에 대해서 말씀드리자면,
저는 값이 바뀌는 건 뷰모델에서 바로 사용하는 편입니다. 중간에 이벤트가 끼게 되면 코드가 길어지게 되어 가독성을 해친다구 생각해서요.

fun updateSignUpUsername(name){
 setState {
               copy(signUpUsername = name)
          }
}

와 같이 하나의 함수에서 처리할 수 있는 것 불필요한 객체를 참조한다고 여겨져서 혹시 Event에 사용 목적이 따로있으신가요

[cacaocoffee]

값이 바뀔 때 다른 무언가 처리를 한다거나 해야한다면 이벤트를 쓰는 것도 좋다고 생각하는데 아직은 잘 안보여서요

[jihyunniiii]

고생하셨습니다

[jihyunniiii]

viewModelScope로 감싼 이유가 있나요?

[jihyunniiii]

1. 텍스트에 입력 받는 값을 상태로 두었으면 해당 상태도 지금처럼 handleEvent 함수 내부에서 업데이트 시켜주시는 것이 올바릅니다. MVI를 사용하는 이유 중 하나가 상태를 한 곳에서 관리해서 상태 변화를 효과적으로 관리하기 위함인데요. (세미나 자료를 참고하시면 좋을 것 같네요.) 이를 효과적으로 처리하기 위해서는 reducer를 통해 상태를 한 곳에서만 변경해주어야 합니다.

[jihyunniiii]

3. 하나의 뷰모델을 여러 뷰에서 나누어서 사용하는 것은 그렇게 권장되는 방식은 아닙니다. 하나의 뷰 모델을 여러 뷰에서 나누어 사용하게 되면 해당 뷰모델을 공유하는 뷰들간 의존성이 커지게 됩니다. 여기서를 예로 들자면 SignIn에만 필요한 코드들이 SignViewModel에 들어가 있어 SignUp 뷰도 이에 대한 불필요한 의존성을 가지게 됩니다. 또한, 이를 합치게 되면서 ViewModel이 방대해지는 문제도 생기고요. (+ 그밖에 다양한 문제들도 있는데 생각해보시면 좋을 것 같습니다.) 따라서 특별한 경우가 아니라면 나누어서 사용하시는 것을 권장합니다.

• 해당 아티클 읽어보셔도 좋을 것 같네요.
  https://blog.gangnamunni.com/post/mvvm_anti_pattern/

[jihyunniiii]

2. 두 방식 다 장단점이 있을 것 같아요. 일단 지금 사용하신 방식은 데이터에 접근할 때에 한 층 더 depth가 있기 때문에 데이터에 접근하는 방식이 조금 더 복잡해집니다. 하지만, UI상태와 데이터를 더 명확히 관리할 수 있죠. (UIState에 따라 이에 맞는 데이터를 가지고 있을 수있으니) 반면, 해당 값을 상태에서 빼게 되면 데이터에 접근하는 것이 더 편리하지만 UI 상태와 데이터를 따로 관리해야 합니다. 각각의 이점에 따라 어떤 방식을 사용하면 좋을지 고민한 뒤 사용하시면 좋을 것 같아요!