[강원대 안드로이드 주민철] 4주차 과제 스텝1 #43
Conversation
라벨과 바텀시트 사용
* Initial commit * 강원대 Android_윤채원 3주차 과제 Step0 (kakao-tech-campus-2nd-step2#17) * Initial commit * style: Make MainActivity.xml * feat: make Place.kt and PlaceDBHelper.kt * feat: add checkPlaceExist of PlaceDBHelper * feat: modify DB files * docs: write readme.md * docs: modify readme.md * feat: consolidate MVVM pattern * docs: write readme.md * style: searchHistory and places cardviews * style: modify recyclerviews * feat: implement viewmodel and livedata * style: modify placemodule * feat: implement searchHistory * feat: add search history animation * feat: implementing search history click * style: move xml ui string to strings.xml * check * delete useless variable * modify code according to feedback * move search history methods to repository * modify README.md * docs: write readme.md --------- Co-authored-by: MyStoryG <[email protected]> * 강원대 Android_윤채원 3주차 과제 Step1 (kakao-tech-campus-2nd-step2#43) * Initial commit * style: Make MainActivity.xml * feat: make Place.kt and PlaceDBHelper.kt * feat: add checkPlaceExist of PlaceDBHelper * feat: modify DB files * docs: write readme.md * docs: modify readme.md * feat: consolidate MVVM pattern * docs: write readme.md * style: searchHistory and places cardviews * style: modify recyclerviews * feat: implement viewmodel and livedata * style: modify placemodule * feat: implement searchHistory * feat: add search history animation * feat: implementing search history click * style: move xml ui string to strings.xml * check * delete useless variable * modify code according to feedback * move search history methods to repository * modify README.md * docs: write readme.md * feat: make DTO and retrofitservice interface * feat: implement api search * style: long place name shortening * refactor: move search api method to repository --------- Co-authored-by: MyStoryG <[email protected]> * 강원대 Android_윤채원 3주차 과제 Step2 (kakao-tech-campus-2nd-step2#89) * Initial commit * style: Make MainActivity.xml * feat: make Place.kt and PlaceDBHelper.kt * feat: add checkPlaceExist of PlaceDBHelper * feat: modify DB files * docs: write readme.md * docs: modify readme.md * feat: consolidate MVVM pattern * docs: write readme.md * style: searchHistory and places cardviews * style: modify recyclerviews * feat: implement viewmodel and livedata * style: modify placemodule * feat: implement searchHistory * feat: add search history animation * feat: implementing search history click * style: move xml ui string to strings.xml * check * delete useless variable * modify code according to feedback * move search history methods to repository * modify README.md * docs: write readme.md * feat: make DTO and retrofitservice interface * feat: implement api search * style: long place name shortening * refactor: move search api method to repository * feat: generate map view methods * docs: write readme.md * feat: add mapview * style: make search window of map activity * feat: clicking search window to go search activity * refactor: modify code according to first feedback * refactor: move history methods to viewmodel --------- Co-authored-by: MyStoryG <[email protected]> * fix: merge rest things * refactor: move places mutable livedata to viewmodel --------- Co-authored-by: MyStoryG <[email protected]>
* Initial commit * 강원대 Android_윤채원 3주차 과제 Step0 (#17) * Initial commit * style: Make MainActivity.xml * feat: make Place.kt and PlaceDBHelper.kt * feat: add checkPlaceExist of PlaceDBHelper * feat: modify DB files * docs: write readme.md * docs: modify readme.md * feat: consolidate MVVM pattern * docs: write readme.md * style: searchHistory and places cardviews * style: modify recyclerviews * feat: implement viewmodel and livedata * style: modify placemodule * feat: implement searchHistory * feat: add search history animation * feat: implementing search history click * style: move xml ui string to strings.xml * check * delete useless variable * modify code according to feedback * move search history methods to repository * modify README.md * docs: write readme.md --------- Co-authored-by: MyStoryG <[email protected]> * 강원대 Android_윤채원 3주차 과제 Step1 (#43) * Initial commit * style: Make MainActivity.xml * feat: make Place.kt and PlaceDBHelper.kt * feat: add checkPlaceExist of PlaceDBHelper * feat: modify DB files * docs: write readme.md * docs: modify readme.md * feat: consolidate MVVM pattern * docs: write readme.md * style: searchHistory and places cardviews * style: modify recyclerviews * feat: implement viewmodel and livedata * style: modify placemodule * feat: implement searchHistory * feat: add search history animation * feat: implementing search history click * style: move xml ui string to strings.xml * check * delete useless variable * modify code according to feedback * move search history methods to repository * modify README.md * docs: write readme.md * feat: make DTO and retrofitservice interface * feat: implement api search * style: long place name shortening * refactor: move search api method to repository --------- Co-authored-by: MyStoryG <[email protected]> * 강원대 Android_윤채원 3주차 과제 Step2 (#89) * Initial commit * style: Make MainActivity.xml * feat: make Place.kt and PlaceDBHelper.kt * feat: add checkPlaceExist of PlaceDBHelper * feat: modify DB files * docs: write readme.md * docs: modify readme.md * feat: consolidate MVVM pattern * docs: write readme.md * style: searchHistory and places cardviews * style: modify recyclerviews * feat: implement viewmodel and livedata * style: modify placemodule * feat: implement searchHistory * feat: add search history animation * feat: implementing search history click * style: move xml ui string to strings.xml * check * delete useless variable * modify code according to feedback * move search history methods to repository * modify README.md * docs: write readme.md * feat: make DTO and retrofitservice interface * feat: implement api search * style: long place name shortening * refactor: move search api method to repository * feat: generate map view methods * docs: write readme.md * feat: add mapview * style: make search window of map activity * feat: clicking search window to go search activity * refactor: modify code according to first feedback * refactor: move history methods to viewmodel --------- Co-authored-by: MyStoryG <[email protected]> * fix: merge rest things * refactor: move places mutable livedata to viewmodel * refactor: implement listadapter * docs: write readme.md * docs: modify readme.md * feat: make addlabel method * style: make map error xml * feat: implementaion of save last position * feat: make bottomsheet methods * style: modify bottomsheet design * refactor: modfify the method of switch activity * fix: remove useless variable --------- Co-authored-by: MyStoryG <[email protected]>
There was a problem hiding this comment.
이번주도 고생하셨습니다!
이전 주차에서 리뷰 했던 내용들이 비슷한 형태로 나오는 부분이 있네요. 변경 부탁드린 부분 전부 고쳐주세요!
현재 객체간 통신을 companion object로 하고 계시는데 이럴 경우 매우 큰 강결합이 생기고 안드로이드 생태계에서는 View들은 각자의 LifeCycle을 갖기 때문에 이런 방식으로 통신하시면 심각한 에러를 초래할 수 있습니다.
컨벤션 부분이나 기본적인 부분들에서 지켜지지 않는 부분들이 꽤 보이네요. 다음 주차 전에는 확실히 고치셔야 합니다! 리뷰 하나하나 다 읽어보시고 의견과 수정 사항 남겨주세요.
리뷰 반영 후 리뷰 Resolve 는 하지 말아주세요. 제가 확인하고 Resolve 하겠습니다.
| private val bottomSheet by lazy { findViewById<LinearLayout>(R.id.bottom_sheet) } | ||
| private val bottomSheetName by lazy { findViewById<TextView>(R.id.name) } | ||
| private val bottomSheetAddress by lazy { findViewById<TextView>(R.id.address) } |
There was a problem hiding this comment.
아직 초기화 되지 않은 시점에서 사용이 되면 UninitializedPropertyAccessException이 발생할 수 있습니다. 또한 액티비티가 소멸된 후에도 뷰가 참조되어 메모리 누수가 발생할 수도 있습니다.
app/src/main/java/campus/tech/kakao/map/adapter/DocumentAdapter.kt
Outdated
Show resolved
Hide resolved
| addWord(document) | ||
| sendDocumentInfo(document) | ||
| documentClicked = true |
There was a problem hiding this comment.
콜백을 묶지 마세요! 콜백 메서드가 여러개가 함께 실행되어야 한다 -> 비즈니스 로직입니다.
판단하는 로직을 View에 두지 마세요
documentCliced 라고 하는 인스턴스 변수를 계속 변경하고 계시는데 객체간 통신은 이런 방식으로 하는게 아닙니다 ㅜㅜ
There was a problem hiding this comment.
잘 이해가 가질 않습니다...ㅠ
다시 setOnClickListener에서 콜백을 호출하게 해야 할까요?
There was a problem hiding this comment.
Adapter 내에서 place를 클릭 하면 addWord 하고, sendDocumentInfo 하고 외부 객체의 인스턴스 변수인 documentClicked에 접근해 값을 설정해주는 이 일련의 동작들이 모두 비즈니스 로직이라는 말씀을 드리려고 했던거였어요.
비즈니스 로직이라 생각되는 부분들은 모두 ViewModel로 옮기셔야 합니다.
There was a problem hiding this comment.
DocumentAdapter 내에 있던 placeClicked 함수를 리팩토링하여 뷰모델 객체를 만들고, 뷰모델로 관련 로직을 옮겼습니다.
뷰모델 내에서는 callback의 onWordAdded와 onDocumentInfoSet 함수를 호출하게 하였으며, 기존 documentClicked 변수는 뷰모델 내 라이브 데이터로 변경하였습니다.
그로 인해 MapActivity에 새로이 라이브데이터 observe를 만들어 검색 결과가 클릭이 되면 라벨(마커)를 생성하고, 그렇지 않으면 제거하게끔 하였습니다.
DocumentAdapter에서 뷰모델 객체를 생성할 때 val viewModel = MainViewModel(MyApplication()) 이렇게 하는 게 맞는지, 또한 뷰모델 내 placeClicked 함수에서 callback 함수들을 호출하는 것이 맞는지 검토 부탁드립니다.
There was a problem hiding this comment.
네 사실 documentClicked는 UI 상태와 관련된 데이터잖아요. 그래서 View를 구성하기 위한 데이터를 관장하는 ViewModel로 그 로직을 옮기신 부분 잘 구현하셨습니다. 다만 아직 부족한 부분이 있는데요. 사용자와 상호작용 하는 부분과 UI State를 어떻게 View와 바인딩 해야 하는지 순서에 대해 이해하실 필요가 있습니다. 아래 설명드리는 순서대로 설계해보세요!
사용자가 RecyclerView 내에서 아이템을 클릭하는 상호작용을 했다고 가정했을 때
- View(Activity)는 상호작용을 인지하고 ViewModel에 관련 로직을 실행하게끔 알려줍니다!
- ViewModel은 비즈니스 로직을 실행하면서 UI에 맞게 데이터를 변경합니다
- 변경된 데이터를 View는 표시합니다.
근데 구현하신걸 보면 클릭 이벤트를 Adapter에 추가해서 Adapter에서 ViewModel을 생성 후 ViewModel의 onPlaceClicked 호출 하고 ViewModel에서는 또 Activity에 구현된 Callback을 통해 View에 구현된 로직 실행하고 또 그 구현 로직들이 ViewModel을 호출하고 있는데 이러면 일원화가 안되는 문제가 있죠.
서면으로 답변 드리기 어려운 부분이 있으니 멘토링 신청해주시면 코드 같이 보면서 설명드리겠습니다
|
더불어서 MVVM 아키텍처 활용이 전혀 안되고 있습니다. 가볍게라도 ViewModel를 설계해보고 데이터가 어떤 방향으로 흘러야 하는지 깊게 고민해보세요. |
|
그리고 MVVM의 패키지 구조에 대해서 생각해보시고 변경부탁드립니다 ~ |
lateinit으로 변경
뷰모델 아키텍처 활용이 전혀 안 되고 있다고 말씀하셨는데, 이번 주차에 추가된 내용이 그렇다는 건가요, 아니면 그 전부터 코딩한 내용들이 다 활용이 안 된 건가요? |
| documentAdapter = DocumentAdapter(this) | ||
| wordAdapter = WordAdapter(this) |
| val addWord: (Document) -> Unit, | ||
| val sendDocumentInfo: (Document) -> Unit | ||
| ): ListAdapter<Document, DocumentAdapter.ViewHolder>( | ||
| object : DiffUtil.ItemCallback<Document>(){ |
전반적인 코드 내용에 대해 말씀드린거였어요. MVVM, 그중 View와 ViewModel 간의 경계에 대해 생각해보시면 좋을 것 같습니다. ViewModel의 역할을 하는 코드들이 View에 많이 보입니다. 각 레이어별 역할에 대해 생각해보시고 구현 로직들을 적절히 옮겨 보시는걸 추천드립니다. |
No description provided.