Conversation
There was a problem hiding this comment.
안녕하세요, 이번 큐시즘 30기에서 Android 파트 심사위원을 맡은 윤여준입니다😊
먼저 정말 고생 많으셨다는 말씀드리며, 코드를 찬찬히 확인해보았고 저도 많이 배울 수 있었습니다. 코드 양이 워낙 방대하고 기간 상 코드의 모든 의도를 파악하고 리뷰를 적을 수 있는 상황은 아닌 것 같아, 이렇게 코멘트로 사전에 질문 주신 내용들에 대한 답변과 제 개인적인 감상을 남겨봅니다.
StateFlow와 mutableStateOf 에 따른 LazyColumn 스크롤 버그 현상
사실 이 부분은 글로 적어주신 것만으로는 완벽하게 이해하기 어려웠는데요 ㅠㅠ
현재 구현해주신 내용으로는 ViewModel 에 선언된 StateFlow 를 collect 하며 LazyColumn을 그려주는 정석적인 방식으로 구현하신 것 같은데, 제가 전달받은 APK 에서는 해당 버그가 재현되지 않아서요. 기존에 문제가 발생했던 방식과, 시도하셨던 방식, 그리고 오픈소스에서 보셨던 예시에 대해 코드로 설명해주시면 더 도움 드릴 수 있도록 하겠습니다!
몇 가지 말씀드릴 수 있는 내용은,
- 사실 StateFlow로 관리하는 상태 변수 또한 Composable State로 변환되어 사용되기 때문에,
결론적으로 mutableStateOf 방식과 동작에 차이가 없을 것으로 보여서 한 쪽에서만 동작이 다르다는게, 그것도 스크롤 속도에 차이가 생긴다는건 직관적으로 생각했을 때는 이해가 안되는 부분이네요 (여러분도 많이 답답하셨겠죠?😂) - 컴포저블의 상태를 ViewModel에서 관리할지, 컴포저블 내에서 state로 관리할지에 대한 기준은, 저의 경우 해당 상태와 Model 간의 의존성을 최우선으로 보는 편입니다. Model 변화에 따라 영향을 받는 state라면 ViewModel에 선언되고, 특정 컴포저블 내에서 단순 UI 상태를 위해 사용된다면 해당 컴포저블 내에 선언할 것 같습니다. 해당 경우에는 말씀 주신대로 model 과의 의존성이 크기 때문에 ViewModel에 선언하는 것이 당연하게 보입니다. 또 고려하는 한 가지는 model과 의존성이 없는 경우라도 하나의 state가 여러 screen에 영향을 주는 경우입니다. MVP와 다르게 MVVM은 여러 개의 View가 하나의 ViewModel 에 의존할 수 있기 때문에, 코드의 복잡도를 낮추기 위해 하나의 ViewModel에서 model과 관계가 없더라도 여러 View에서 동기화되어야 하는 state 를 관리하기도 합니다.
전역 BottomSheet 관리
탭 화면에서 표시하는 BottomSheet 가 Navigation Bar 를 가리는 디자인으로 나와 고충을 겪는 건 저도 자주 겪어본 상황인데요😅 우선 Main과 Backlog 간의 결합도를 낮추기 위해 ViewModel 을 분리하고 이벤트를 주고 받는 방식으로 노력하신 부분에 대해선 코드의 복잡도가 다소 늘더라도 훌륭한 방안이라고 생각합니다🙌 우선 개발적인 부분을 배제하고 오직 UI 디자인을 통해서 컴포넌트 간의 위계를 생각해 볼 필요가 있는데요. 엄밀히 말하면 MainScreen과 BacklogScreen은 같은 Screen이라는 이유로 위계가 같아보일 수 있지만, 사실 BacklogScreen은 메인 탭 중 하나기 때문에 MainScreen 보다 하위 개념으로, 어느정도 종속성이 있을 수 있습니다.
따라서 두 화면의 결합도에 대해서 조금은 긴장을 푸셔도 좋다고 말씀 드리고 싶은데요! 다만 말씀 주셨던 ViewModel 을 공유하는 방법은 너무 결합도가 강해져 피하면 좋을 것 같고, 대신 아래와 같은 방법들에 대해 고민해도 좋을 것 같습니다.
- 간단히 생각해보면 하위(Backlog)에서 상위(Main)로 상태를 변경하는 것이므로, hoisting 개념을 사용하는 것이 제일 깔끔할 것 같습니다. Main에서 BottomSheet 상태를 갖고, Backlog에
onShowBottomSheet: (BottomSheetState) -> Unit같은 파라미터를 전달하는 방법이 있을 것 같습니다. - 화면 결합도가 높아질 수 있지만, 코드 복잡도를 낮추는 방법으로는 MainViewModel에서 BottomSheet 상태를 관리하고 BackLogScreen에서 싱글톤으로 선언된 MainViewModel을 직접 참조하는 방법도 있을 것 같습니다.
snapshotList 에 대한 잠재적인 문제점?
구현해주신 리스트 업데이트 방식은 UI 상에서는 유저 액션에 따라 리스트의 상태 변화를 즉각적으로 반영하고, 비동기 요청이 실패했을 때만 UI를 원상 복구 시켜 UX를 높이는 방향으로 정말 잘 구현해주신 것 같습니다. 다만 UI의 동작이 서버 환경에 너무 크게 의존한다는 문제점이 있을 수 있는데요! 일반적인 상황에서의 문제는 아니고 극단적인 경우입니다만, 예를 들어 서버 응답이 매우 느리거나 서버 환경에 비해 한 번에 매우 많은 유저 액션이 들어올 때 유저의 예상과 다르게 리스트가 업데이트 되는 경우가 있을 것 같습니다.
생각나는 개선 방법 중 하나는, 리스트 UI와 서버 간 데이터 업데이트를 최대한 단방향으로만 진행하는 것이 있을 것 같은데요~ snapshotList 업데이트는 서버 요청 실패 시에만 진행하고, 성공 시에는 서버 기준 데이터 업데이트 없이 사용자 액션에 의해 변화한 list를 그냥 유지하는 것은 어떨까요?
클린 아키텍쳐, 멀티 모듈, Gradle 플러그인
UseCase 에 기반한 클린 아키텍쳐를 멀티 모듈을 통해 구현해주셨는데요.
우선 완성도와 노고에 대해 고생하셨다는 말씀 드리고 싶습니다👍
유즈케이스 세분화, 모듈 간 의존성 방향, 데이터와 UI 간 단방향 흐름, 공통 클래스 및 컴포넌트 추상화 모두 잘해주셨습니다. 다만 이런 방식으로 개발하면서 가장 중요한 것은 바로 여러분이 어떤 걸 느꼈는지라고 생각하는데요! 클린 아키텍쳐를 통해 정말 유지보수가 쉬워졌는지, 모듈화를 통해 코드 재사용성이 늘었는지, 아니면 괜히 생산성만 떨어진 게 아닌지, 이것이 이 프로젝트에 가장 적합한 아키텍쳐였는지 스스로 회고해보면 좋을 것 같습니다. 저는 처음으로 클린 아키텍쳐를 겪어본 뒤 충분히 고민한 뒤 적용에 대한 나름의 기준을 세웠었는데, 이 과정이 많이 도움됐었어서요!
그 외에 Compose 를 recomposition을 고려하며 깊이 있게 사용한 점도 굉장히 인상 깊었습니다😊
현업에서는 보통 개발하고 유지보수하는 과정에서 재사용이 많이 되는 특정 코드들이 발견되거나, 배포를 따로 해야될 필요가 있는 코드의 경우 모듈화를 진행하는 편입니다. 따라서 멀티 모듈 구조는 굉장히 흔하게 사용하는 편이지만, 그것을 스스로 클린 아키텍쳐라고 칭하는 팀은 많이 못 본 것 같습니다. Gradle 커스텀 플러그인도 여러 모듈의 gradle 에서 사용하는 플러그인을 관리하기 위해 많이 사용하는 편입니다. 물론 전부 다 회바회 팀바팀입니다~😅
그 외에 자잘한 것
현재 UseCase의 invoke operator를 오버라이딩해서 사용하고 계신데요. 그럼에도 코드 상에서는 GetTokenUseCase.invoke() 와 같이 사용하고 계신 것 같아서요~ 오버라이딩 하신 만큼 GetTokenUseCase() 와 같이 사용하면 코드 스타일 개선에 도움이 될 것 같습니다!
신입 개발자로서의 역량
직무에 관계 없이 어느 곳이나 원하는 신입의 역량은 바로 "기초"라고 생각합니다. 당장은 취준생들과 여러 프로젝트형 동아리 사이에서는 Compose나 클린 아키텍쳐, 또는 hilt 나 retrofit 같은 오픈소스에 열중하지만, 막상 면접에 들어가면 Android의 컴포넌트 별 상세한 생명주기, Handler나 Looper의 동작, View가 그려지는 원리 등과 같이 제쳐두고 있었던 기본 개념을 묻는 경우가 많습니다. 신기술에 집중하는 것도 좋지만, 언제나 기본을 잃지 않고 준비한다면 좋은 결과가 있을거라고 생각합니다~👍
조금 서두없이 쓴 부분도 있고, 수없이 고민한 여러분의 입장에선 미숙한 답변도 있을 것 같습니다😂
조금이라도 도움이 되었으면 하는 바램이며 추가적으로 여쭤보고 싶은신 게 있다면 또 질문 주셔도 좋을 것 같습니다.
밋업 데이까지 응원하겠습니다~! 감사합니다 :)
…ay-todo Refactor: 어제 한 일 페이지 로직 수정
…-token Refactor: 토큰 갱신 api 요청 data class 수정
Feat/126 deadline mode
…ompletion Feat: 어제 한 일 완료 api 연결
…sheet-content-clickable Refactor: 바텀시트 내부 클릭 이벤트 처리 로직 수정
…d-drop Refactor: 드래그앤드롭 로직 개선
…quest-body Refactor: 로그아웃, 리프레시 API 요청 바디 수정
…nticator Fix: 토큰 갱신 로직 수정
이슈 번호
작업내용
코드리뷰용 PR
결과물
리뷰어에게 추가로 요구하는 사항(선택)