-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
코드 리뷰에 있어서 질문 드리고 싶은 부분 정리 #74
Comments
|
|
|
|
@taehwandev 답변 감사드립니다! SpeakerDeck에 최근에 발표하신 What’s new in Android? 발표자료를 통해 말씀하신 2022년, 23년 발표자료가 무엇인지 찾았습니다. 꼭 확인해보도록 하겠습니다! 또한 UiState를 구현하는 방식을 data class 에서 sealed interface를 이용하는 방법으로 각 상태별로 어떻게 나눠야 좋을지 고민해보도록 하겠습니다! ㅎㅎ |
@taehwandev |
@easyhooon @likppi10 NIA를 기반으로 나만의 코드를 만드시면 좋습니다! 두분이서 함께 하고 있다보니 설계에 대한 고민을 하실 수 있어 좋아보이고요. 근데 여기서 중요한건 UseCase 구글 가이드상으론 단순 포워딩 할 때는 사용하지 않고, 두개의 Repository를 합치는 경우에 활용합니다. 이런것들도 고민이 필요하고, 사실 모든게 다 고민이 필요해요. 고민 없이 따라하는건 쉽지만, 이후부턴 전부다는 아니고, 하나둘 정도 궁금해하시면서 조금 더 고민해보면 좀 더 좋은 코드가 나올 수 있으실겁니다.(당연히 최고는 고민 없이 작성할 수 있어야 좋죠) |
전체적으로 프로젝트의 구조는 now in android를 참고하여 모듈화를 진행하였습니다.
본 프로젝트는 di를 core/data 모듈에서 진행하고 있는데, app 모듈에서 di를 진행하는 것이 더 나은 방법인 것인지, 이대로도 괜찮은지 궁금합니다.(이석규)
core/data/di
ViewModel에서 관리 중인 UiState가 앱의 기능이 추가됨에 있어 파라미터들이 하나씩 추가되어, 하나의 UiState 자체가 비대해지는 문제가 발생하였는데, 이것을 어떤 기준, 방법을 통해서 조금이나마 분리하여, 나눌 수 있을지, 이대로도 괜찮은 지 궁금합니다.
UiState 를 data class 로 선언하여 관리할때는 그 화면이 간단할때만 해야된다는걸 체감하게 되는 것 같슴니다..(이지훈, 이석규)
HomeViewModel.kt
BandalartBottomSheetViewModel.kt
표를 보여주는 HomScreen 컴포저블은 어느정도 함수의 모듈화가 이루어졌다고 생각하는데, BottomSheet를 이용하는 부분은 무수한 spacer와 image가 card에 둘러싸여 있는 등 날것의 컴포저블들이 상당 수가 BandalartBottomSheet 내부에 그대로 포함 되어있습니다. 바텀 시트를 보셨을 때, 어떻게 함수를 분리하면 좋을지, 생각하시는 나뉠수 있는 기준, 방법이 궁금합니다.(이석규)
HomeScreen.kt
BandalartChart.kt
BandalartBottomSheet
앱에 대한 기능을 테스트해보면서 홈화면에서 불필요하게 많이 Recomposition이 수행되는걸 확인할 수 있었는데(표를 불러오는 과정에서 TopBar 에 반다라트라는 단어가 계속 깜빡거리는 등의..) Recomposition 을 현 상황에서 어떻게 최적화시킬수 있을지, 최적화 시킬 수 있는 부분이 존재하는지 궁금합니다! (이런 부분을 찾아내서 최적화 하는 것이 진정한 실력있는 개발자인데, 아직 개선시킬 수 있는 부분을 잘 찾지 못하겠습니다...)(이지훈)
HomeScreen.kt
HomeViewModel.kt
또한 뷰모델의 특정한 하나의 함수가 너무나도 많은 역할을 수행하고 있고, 이것이 좋지 못한 방법이라는 것을 인지하고 있어, 개선해보도록 하겠습니다..
코드를 확인해주셔서, 그리고 리뷰해주셔서 감사합니다!
The text was updated successfully, but these errors were encountered: