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주차 과제_2단계 #83

Open
wants to merge 109 commits into
base: lovelhee
Choose a base branch
from

Conversation

lovelhee
Copy link

@lovelhee lovelhee commented Jul 26, 2024

안녕하세요 멘토님!
배려해주신 덕분에 어제 회의는 잘 마무리 짓고 왔습니다😢. 감사합니다 :)


2단계 커밋 모음

https://github.com/kakao-tech-campus-2nd-step2/android-map-refactoring/pull/83/files/d876e3d208e8ac5278a53511d77593578cbcc066..ed93acaed102410db356d833b01c54683e8b59d3


중점적으로 봐주셨으면 하는 부분

  • MVVM 적용하면서 자료를 보고 커스텀 어댑터가 필수(?)인줄 알았는데 아니더라구요 .. ! 그래서 처음에 많이 헤매고 꼬였던 거 같아서 불필요한 부분이 남아있거나 복잡할 것이라고 생각됩니다🙀. 혹시 놓친 부분은 없는지 부족한 점이나 개선점에 대해 편하게 말씀해주세요!

마무리 및 질문

  • 저번 주차를 진행하면서 패키지 분리에 대한 고민을 하는 요즘입니다 .. ㅎㅎ 만약에 커스텀 어댑터를 사용한다면, 해당 어댑터는 ui 패키지가 맞을지, 따로 패키지(util이나 adapter 등)를 만들어서 새로 배치할지에 대해 생각을 해봤습니다. 저는 xml에 사용되는 부분이다보니, ui가 맞지않나라는 생각도 들고, 또 화면에 나타나는 부분은 아니니 util이나 adapter 같은 패키지가 맞나싶은 생각도 듭니다. 이에 대해 멘토님의 의견이 궁금합니다!

InyoungAum pushed a commit that referenced this pull request Jul 26, 2024
* Initial commit

* feat: step0 (#17)

* 충남대Android_전영주_4주차 과제_Step1 (피드백 반영) (#37)

* feat: step0

* docs: 1단계 기능 목록 작성

* feat(ui): create BottomSheet layout

* feat: 마커 기능&바텀 시트 추가

* feat: 에러 화면 추가

* feat: 마지막 위치를 저장 및 앱 실행 시 포커스

* feat: 피드백 반영

* 충남대 Android_전영주 4주차 Step2 (#83)

* feat: step0

* docs: 1단계 기능 목록 작성

* feat(ui): create BottomSheet layout

* feat: 마커 기능&바텀 시트 추가

* feat: 에러 화면 추가

* feat: 마지막 위치를 저장 및 앱 실행 시 포커스

* feat: 피드백 반영

* feat: step2 UI테스트 코드 추가

* step0: 4주차 코드 가져오기

* feat: add Room entity annotation to DocumentEntity

* feat: add SearchQueryEntity

* refactor: restructure project folders

* refactor: refactor to use Room and Hilt for DI

- SearchQueryDao, VisitedPlaceDao 및 SearchQueryEntity, VisitedPlaceEntity 를
생성
- SharedPref 대신 Room 데이터베이스를 사용하도록 함
- 프로젝트 전반에 Hilt를 구현
- Hilt 사용을 위해 의존성 제공 모듈인 DatabaseModule, RepositoryModule,
	UsecaseModule 생성
- 메인 스레드 차단 방지를 위해 DB 작업은 백그라운드로 이동
- 원래 기능 별로 커밋하려 했으나 에러를 고치는 과정에서 다 연관되어 있어 한
번에 커밋합니다. ㅠ

* fix: fix RecyclerView item click listener error

- 아이템 클릭 리스너가 작동하지 않은 문제 해결
- 원인: XML 레이아웃 파일에 clickListener 바인딩이 누락됨

* fix: update search history order and prevent duplicate entries

- 검색 기록 목록의 순서를 최신 검색이 가장 왼쪽에 오도록
변경했습니다.(reversed 사용)
- 중복된 검색 기록이 추가되지 않도록 수정했습니다.

* save the project

* chore: recommit to correct previous commit

* fix: 데이터 바인딩 관련 피드백 반영

* refactor: Update DI configuration for PlaceRepository and UseCases using Hilt

- 기존에는 Provide 를 사용여서 의존성 주입을 하였으나 interface, Impl 을 @BINDS
를 통해 bind 하는 방법으로 변경했습니다
- 비동기 작업 처리를 위해 repo Impl 에 디스패처 제공하였는데 object
CoroutineDispatcherModule이 없으면 순환 참조 에러가 발생하여 이를
추가하였습니다. 이에 대한 추가적 공부를 주말에 해보겠습니다.

* refactor: restructure project folders

- data 패키지 내의 usecase 구현체를 domain 모듈로 이동했습니다
- API 서버로 부터 받게 되는 폴더를 entity 로 변경하고 로컬에서 데이터 전달 시
사용되는 domain 모듈 내의 model 을 dto 로 변경하였습니다

* refactor: add Qualifier for Database Name String injection

- 데이터베이스 문자열 주입을 위해 @qualifier 을 사용하였습니다
- 아직 잘 모르는 개념이라 시간이 부족하여 주말동안 더 조사해보겠습니다.

* refactor: inject HttpService using Hilt

- HttpService 도 의존성 주입을 사용하였습니다.
- APIKey 와 BaseUrl 을 네트워크 모듈내에서 제공하여 HttpService 를 provide
하였습니다.

* save the project

---------

Co-authored-by: MyStoryG <[email protected]>
@LeeOhHyung LeeOhHyung self-requested a review July 28, 2024 14:17
@LeeOhHyung LeeOhHyung assigned LeeOhHyung and lovelhee and unassigned LeeOhHyung Jul 28, 2024
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~ 5주차도 고생많으셨습니다.
BindingAdapter는 MVVM 아키텍처를 구현하기 위한 필수재 라기 보다는요. 데이터바인딩 라이브러리를 더 잘 활용하기 위해서 XML에 직성 속성들을 만들어주고 싶을때 사용할 수 있습니다. 동일한 코드를 Activity/Fragment/ViewModel 에서 구현할 수 도 있지만, 별도의 바인딩어댑터를 만들어서 코드를 줄이거나, 여러 XML에서 재사용할 수 있도록 만들어줄수도 있죠. 적절하게 활용해보면 좋습니다

Comment on lines 44 to +50
// 검색 기능이 잘 동작하나요?
@Test
fun testVerifySearchFunction() {
scenario.onActivity { activity ->
val keyword = "london"
val editText = activity.findViewById<EditText>(R.id.etKeywords)
editText.setText(keyword)

activity.mapViewModel.searchPlaces(keyword)

activity.mapViewModel.searchResults.observe(activity, Observer { results ->
assertTrue(results.isNotEmpty())
})
}
}

// 검색 결과에 따라 RecyclerView가 잘 바뀌나요?
@Test
fun testRecyclerViewVisibility() {
scenario.onActivity { activity ->
val recyclerView = activity.findViewById<RecyclerView>(R.id.rvSearchResult)
val noResultTextView = activity.findViewById<TextView>(R.id.tvNoResults)

activity.mapViewModel.searchResults.observe(activity, Observer { results ->
if (results.isEmpty()) {
assertEquals(View.VISIBLE, noResultTextView.visibility)
assertEquals(View.GONE, recyclerView.visibility)
} else {
assertEquals(View.GONE, noResultTextView.visibility)
assertEquals(View.VISIBLE, recyclerView.visibility)
}
})
}
}
// @Test
// fun testVerifySearchFunction() {
// scenario.onActivity { activity ->
// val keyword = "london"
// val editText = activity.findViewById<EditText>(R.id.etKeywords)
// editText.setText(keyword)

Choose a reason for hiding this comment

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

테스트코드가 모두 주석처리 되어있네요.
동작변경으로 인해 새롭게 작성해야해서 그런것인가요??

Copy link
Author

Choose a reason for hiding this comment

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

MainActivty.kt 를 변경하면서 해당 테스트 빌드시 오류가 나길래 잠시 주석처리해뒀습니다 - !! 추후에 테스트도 리팩토링 해보려구요!

Comment on lines +9 to +19
//@BindingAdapter("app:keyword")
//fun setKeyword(editText: EditText, keyword: MutableLiveData<String>?) {
// if (keyword != null && editText.text.toString() != keyword.value) {
// editText.setText(keyword.value)
// }
//}
//
//@InverseBindingAdapter(attribute = "app:keyword")
//fun getKeyword(editText: EditText): String {
// return editText.text.toString()
//}

Choose a reason for hiding this comment

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

아마 XML EditText에 양방향 데이터바인딩 을 적용하기위한 코드로 보이네요. MutableLiveData 타입으로 선언후 XML에서 android:text=@={viewModel.keyword} 형태로 사용하면 내부적으로 자동으로 two-way databinding이 적용되게 됩니다. 참고하여 사용하시면 될것 같습니다

Copy link
Author

Choose a reason for hiding this comment

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

양방향을 적용시키기 위함이 맞습니다! 처음에 코드를 짤 때 해당 오류가 계속 발생해서 자료를 찾아보니 BindingAdapter를 사용하는 것이 해결 방법이라고 해서 모든 xml 바인딩에 커스텀 어댑터가 필수적이라고 오해했어요 😢 멘토님이 알려주신 방법이 훨씬 좋은 거 같습니다 👍👍

Comment on lines +36 to +40
binding.rvSearchResult.layoutManager = LinearLayoutManager(this)
binding.rvSearchResult.adapter = searchResultAdapter

binding.rvKeywords.layoutManager = LinearLayoutManager(this, LinearLayoutManager.HORIZONTAL, false)
binding.rvKeywords.adapter = keywordAdapter

Choose a reason for hiding this comment

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

RecyclerView에 app:layoutManager="..." 속성으로 LayoutManager를 설정해줄 수 있습니다. 지금의 형태는 커스텀한 LayoutManager를 사용하는것이 아니기 때문에 XML속성으로도 사용가능해보이네요. 물론 Activity에서 하더라도 문제가 있는것은 아닙니다. UI 설정을 XML에서도 할 수 있는 방법이기때문에 참고하시면 좋을것 같습니다

Copy link
Author

@lovelhee lovelhee Jul 29, 2024

Choose a reason for hiding this comment

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

오! 그 방법으로 한 번 적용해보겠습니다 !

Comment on lines +154 to +156
)
bottomSheetBinding.lifecycleOwner = this
bottomSheetBinding.viewModel = mapViewModel

Choose a reason for hiding this comment

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

bottomSheetBinding.lifecycleOwner = this 이렇게 했을때 this가 MapActivity로 걸리게 될것 같습니다.
바텀시트 다이얼로그 또한 LifecycleOwner 인터페이스를 상속 받고 있기 때문에 bottomSheetDialog를 넣어주는것이 조금더 적절할것 같습니다

Copy link
Author

Choose a reason for hiding this comment

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

넵! bottomSheetDialog를 넣어주는 방향으로 수정해보겠습니다.

Comment on lines +167 to +170
errorBinding.message = message
errorBinding.root.visibility = View.VISIBLE
binding.searchLayout.visibility = View.GONE
binding.mapView.visibility = View.GONE

Choose a reason for hiding this comment

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

errorBinding의 binding variable로 message를 등록하고 있으니, message 값이 있는지에 따라 XML에서 VISIBLE/GONE 처리도 할 수 있지 않을까 싶네요. 이건 개선사항이라 Activity에서 하는게 더 편하시다면 그대로 사용하셔도 무방하긴합니다

Copy link
Author

Choose a reason for hiding this comment

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

넵! 함께 시도해서 6주차 PR 0단계때 커밋 올리겠습니다 :)

Comment on lines +116 to +117
viewModelScope.launch {
val sharedPreferences = getApplication<Application>().getSharedPreferences("keywords", Context.MODE_PRIVATE)

Choose a reason for hiding this comment

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

SharedPreference도 기기에 저장된 데이터를 가져온다는 관점에서 data layer에 있는게 적절할듯 합니다. 따라서 repository를 통해서 원하는 값을 가져오거나 / 값을 설정하도록 변경해보면 어떨까요.

dao처럼 Repository의 생성자를 통해서 전달받도록 만들수 있을겁니다!
hilt module에서 추가적인 선언도 필요하겠네요

Copy link
Author

Choose a reason for hiding this comment

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

넵! 수정해보겠습니당

InyoungAum pushed a commit that referenced this pull request Jul 30, 2024
* Initial commit

* feat: step0 (#17)

* 충남대Android_전영주_4주차 과제_Step1 (피드백 반영) (#37)

* feat: step0

* docs: 1단계 기능 목록 작성

* feat(ui): create BottomSheet layout

* feat: 마커 기능&바텀 시트 추가

* feat: 에러 화면 추가

* feat: 마지막 위치를 저장 및 앱 실행 시 포커스

* feat: 피드백 반영

* 충남대 Android_전영주 4주차 Step2 (#83)

* feat: step0

* docs: 1단계 기능 목록 작성

* feat(ui): create BottomSheet layout

* feat: 마커 기능&바텀 시트 추가

* feat: 에러 화면 추가

* feat: 마지막 위치를 저장 및 앱 실행 시 포커스

* feat: 피드백 반영

* feat: step2 UI테스트 코드 추가

* step0: 4주차 코드 가져오기

* feat: add Room entity annotation to DocumentEntity

* feat: add SearchQueryEntity

* refactor: restructure project folders

* refactor: refactor to use Room and Hilt for DI

- SearchQueryDao, VisitedPlaceDao 및 SearchQueryEntity, VisitedPlaceEntity 를
생성
- SharedPref 대신 Room 데이터베이스를 사용하도록 함
- 프로젝트 전반에 Hilt를 구현
- Hilt 사용을 위해 의존성 제공 모듈인 DatabaseModule, RepositoryModule,
	UsecaseModule 생성
- 메인 스레드 차단 방지를 위해 DB 작업은 백그라운드로 이동
- 원래 기능 별로 커밋하려 했으나 에러를 고치는 과정에서 다 연관되어 있어 한
번에 커밋합니다. ㅠ

* fix: fix RecyclerView item click listener error

- 아이템 클릭 리스너가 작동하지 않은 문제 해결
- 원인: XML 레이아웃 파일에 clickListener 바인딩이 누락됨

* fix: update search history order and prevent duplicate entries

- 검색 기록 목록의 순서를 최신 검색이 가장 왼쪽에 오도록
변경했습니다.(reversed 사용)
- 중복된 검색 기록이 추가되지 않도록 수정했습니다.

* save the project

* chore: recommit to correct previous commit

* fix: 데이터 바인딩 관련 피드백 반영

* refactor: Update DI configuration for PlaceRepository and UseCases using Hilt

- 기존에는 Provide 를 사용여서 의존성 주입을 하였으나 interface, Impl 을 @BINDS
를 통해 bind 하는 방법으로 변경했습니다
- 비동기 작업 처리를 위해 repo Impl 에 디스패처 제공하였는데 object
CoroutineDispatcherModule이 없으면 순환 참조 에러가 발생하여 이를
추가하였습니다. 이에 대한 추가적 공부를 주말에 해보겠습니다.

* refactor: restructure project folders

- data 패키지 내의 usecase 구현체를 domain 모듈로 이동했습니다
- API 서버로 부터 받게 되는 폴더를 entity 로 변경하고 로컬에서 데이터 전달 시
사용되는 domain 모듈 내의 model 을 dto 로 변경하였습니다

* refactor: add Qualifier for Database Name String injection

- 데이터베이스 문자열 주입을 위해 @qualifier 을 사용하였습니다
- 아직 잘 모르는 개념이라 시간이 부족하여 주말동안 더 조사해보겠습니다.

* refactor: inject HttpService using Hilt

- HttpService 도 의존성 주입을 사용하였습니다.
- APIKey 와 BaseUrl 을 네트워크 모듈내에서 제공하여 HttpService 를 provide
하였습니다.

* save the project

* fix: fix lastPlace update error

- getLastPlace 에서 postValue 를 사용하면 정상적으로 마지막 데이터를 가져오지
않아서  _lastPlace.value = place 로 수정하였습니다. 또한, runBlocking 으로
마지막 장소를 동기적으로 가져왔더니 해결되었습니다.

* feat: add Mapper class for clean code

- 코드 가독성을 위해 utils/place

* feat: add Mapper class for clean code

- 코드 가독성을 위한 매퍼 클래스 추가:

* refactor: Mapper 관련 피드백 반영

* refactor: RecyclerView 데이터 바인딩 적용

---------

Co-authored-by: MyStoryG <[email protected]>
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