Skip to content
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

부산대 Android_김현민 5주차 과제 Step2 #69

Open
wants to merge 82 commits into
base: khyeonm
Choose a base branch
from

Conversation

khyeonm
Copy link

@khyeonm khyeonm commented Jul 26, 2024

Step2 커밋 링크

(Step2 커밋만 select 하였습니다!)
https://github.com/kakao-tech-campus-2nd-step2/android-map-refactoring/pull/69/files/59c928d0e692a0145af1e559d807abd57e882fa3..261f6a36a1f9245616378c14a20926140a0b00aa

느낀 점

  • 간단한 Activity에서의 MVVM 적용은 수월했지만 MainActivity에서 MVVM 적용이 너무 어려웠다.

질문드리고 싶은 점

  • DataBinding과 LiveData를 적절하게 잘 사용하여 MVVM 아키텍처 적용이 잘 되었는지 궁금합니다. (특히 MainActivity 관련!!)

khyeonm and others added 27 commits July 19, 2024 17:07
* Initial commit

* Docs: Update README.md

* Feat: Complete search screen layout

* Feat: Add data to the database

* Docs: Update README.md

* Feat: Create item_view

* Feat: Modify main view

* Feat: Implement search list

* Feat: Complete saved element view

* Feat: Complete save action

* Feat: Add persistent save function

* 부산대 Android_김현민_2주차_과제_Step1 (kakao-tech-campus-2nd-step2#2)

* Docs: Update README.md

* Feat: Complete search screen layout

* Feat: Add data to the database

* Design: Modify save element design

* Update README.md

* Initial commit

* Fix: Fix Week2 error

* Docs: Update README.md

Step1 README

* Feat: Add initial settings

* Feat: Add Document data class

* Feat: Add RetrofitService interface

* Refactor: Delete DB code

* Feat: Add search function using API

* Feat: Separate CategoryGroupCode

* Docs: Update step2 README.md

* Feat: complete initial project setup

* Feat: Add mapview and map function

* Fix: resolve errors

* Chore: Update network security configuration

* Fix: Resolve windows emulator issue

* Style: Update mapview design

* 부산대 Android_김현민 3주차 과제 Step0 (kakao-tech-campus-2nd-step2#19)

* Initial commit

* Docs: Update README.md

* Feat: Complete search screen layout

* Feat: Add data to the database

* Docs: Update README.md

* Feat: Create item_view

* Feat: Modify main view

* Feat: Implement search list

* Feat: Complete saved element view

* Feat: Complete save action

* Feat: Add persistent save function

* 부산대 Android_김현민_2주차_과제_Step1 (kakao-tech-campus-2nd-step2#2)

* Docs: Update README.md

* Feat: Complete search screen layout

* Feat: Add data to the database

* Design: Modify save element design

* Update README.md

---------

Co-authored-by: MyStoryG <[email protected]>

* Fix: Reflect feedback

* Fix: Resolve error

* Docs: Update README.md

* Feat: Add location marker

* Feat: Add camera movement function

* Feat: Add bottom sheet dialog

* Feat: Add initial camera location function

* Refactor: Resolve initial camera issue and search issue

* Feat: Add map error event

* Design: Modify UI design

* Refactor: Separate function and write annotations

* Docs: Update README.md

* Feat: Add save item click function

* Feat: Add keyword search function

* Update README.md

---------

Co-authored-by: MyStoryG <[email protected]>
dependency injection about network
dependency injection about map utility
dependency injection about map search service
dependency injection about search save service
dependency injection about bottomsheet
Resolve errors caused by code modification during refactoring process
Utilize LiveData and DataBinding
Utilize LiveData and DataBinding
Utilize LiveData and DataBinding
@LeeOhHyung LeeOhHyung self-requested a review July 28, 2024 15:45
Copy link

@LeeOhHyung LeeOhHyung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM~ step1 코멘트도 모두 잘 반영해주셨네요.
step2는 크게 코멘트가 필요없을 정도로 코드작성 해주셨어요. 5주차 과제도 고생많으셨습니다.

Comment on lines +47 to +59

private fun handleSearchResult(result: KakaoResponse?) {
result?.documents?.let { documents ->
if (documents.isEmpty()) {
_noResult.value = true
} else {
val profilesList = documents.map { it.toProfile() }
_profiles.value = profilesList
}
} ?: run {
_noResult.value = true
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

network layer에서 결과를 전달받을때 response 전체를 받기보다는 원하는 데이터인 documents만 전달받는게 좀더 깔끔할것 같습니다! UI layer에서 원하는것은 api응답 이라기 보다는 확인하고자 하는 document 데이터 이기 때문이죠!

그렇게 되면 searchService 를 중간에서 확인해 documents만 전달해주는 중간 전달자를 두는 방법이 있겠죠. 보통은 repository나 usecase 라는 이름을 사용해서 만들고 있습니다

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

KakaoResponse 라는 클래스가 data 에도 있고, network 패키지에도 있네요. 둘중 하나는 제거해도 될것 같습니다

Comment on lines +53 to +56
lifecycleScope.launch {
withContext(Dispatchers.IO) {
db.profileDao().insertAll(*profiles.toTypedArray())
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아하.. database에 데이터는 insert 하는것은 ui에서 수행하기 보다는 data layer에 있는 것이 적절합니다.
따라서, observe 할때 수행되기 보다는 ViewModel 에서 profiles livedata에 값을 전달해주는 시점에 바로 db insert 시키는 방법이 더 좋을것 같습니다. Activity에는 데이터를 다루는 로직보다는 정말 UI와 관련된 부분만 존재하게 되는것이죠

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants