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_권성찬 2주차 과제 Step2 #46

Open
wants to merge 45 commits into
base: ksc1008
Choose a base branch
from

Conversation

ksc1008
Copy link

@ksc1008 ksc1008 commented Jul 5, 2024

코드작성 시, 어려웠던 점

이번 과제를 수행하면서 많은 내용을 처음 접해보고 새로 배우면서 진행한 것 같습니다.
먼저, SQLite를 이용해 DB에 접근해보는 것은 처음입니다. 또한 MVVM을 이용한 안드로이드 앱 개발 또한 경험이 전무했습니다.
MVVM에 있어서, 뷰와 뷰 모델, 그리고 모델 간의 책임을 적절하게 분배하고 상호간의 인터페이스를 설계하는게 쉽지 않았던 것 같습니다.

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

  1. 모델 클래스에서 컨텍스트를 활용해야 하는 상황이 많이 발생했습니다. 가능하면 애플리케이션 레이어와 도메인 레이어 간 결합도를 낮추고 싶지만, DB에 접근하기 위해서는 컨텍스트를 위임해야 해서 불가피하게 참조를 하였습니다. 모델 클래스를 초기화할 때 Context객체를 전달해 초기화하는 방법 외에 더 나은 방식이 있을지 궁금합니다.
  2. 액티비티에서는 가능한 레이아웃 생성 및 뷰모델 바인딩과 관련된 역할만 수행하게 하고 싶었습니다. 하지만 아직 액티비티에서 애플리케이션의 특정 상태를 조작하는 기능을 일부 수행하는 것 같습니다. 외부 요인에 의해 상태가 변경되는, 아주 사소한 값 하나 하나라도 모두 뷰 모델에 정의하고 액티비티에서는 이를 observe 하는 방식으로 가야 할 지 궁금합니다.
  3. 강사님께서 리사이클러 뷰 어댑터를 설계할 때 추상화에 신경쓰라고 말씀하셨습니다. 특히나 검색어 저장과 관련된 SearchKeywordAdapter같은 경우 각 아이템 별로 리스너가 두 개 (클릭, 삭제버튼 클릭)나 존재해서 구현에 곤혹을 겪었습니다. 강사님이 가이드하신 대로 리스너를 외부에 노출시키고, 클래스 내부에서 독자적인 로직을 수행하지 않고 뷰를 재정의하지 않는 방향으로 코드를 짰는데, 더 개선시킬 방법이 있을까요?
  4. DB를 써본 경험은 거의 없었던 것 같습니다. 제가 작성한 Contract, SQLiteOpenHelper 클래스가 요구사항에 맞게 적절히 작성되었는지, SQLiteOpenHelper와 레포지토리, 뷰 모델의 책임 분배가 적절히 되었는지 궁금합니다.

구현 화면

Screenshot_20240705_172732_Map
Screenshot_20240705_172749_Map

ksc1008 added 30 commits July 1, 2024 13:38
SearchResultRepository: 데이터베이스와 통신하여 Observable한 검색 결과를 저장하는 레포지토리
SearchActivityViewModel: SearchResultRepository의 LiveData를 위임받아서 검색 결과를 자동으로 갱신하고, 유저 입력에 따라 레포지토리를 통해 DB 쿼리를 수행하는 ViewModel
ViewModel의 검색 결과 값을 observe하여 RecyclerView에 반영하는 역할을 수행
…licts

SearchKeywordContract: 검색 키워드를 저장하는 테이블에 대한 스키마
SearchKeywordList: DB에 저장된 search keywords를 보여줍니다. 클릭 시 해당 검색어를 검색합니다.
@ksc1008 ksc1008 marked this pull request as draft July 5, 2024 07:55
@ksc1008 ksc1008 marked this pull request as ready for review July 5, 2024 08:22
Copy link

@mkSpace mkSpace left a comment

Choose a reason for hiding this comment

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

코드를 잘 작성해주셨네요. 이번주도 수고 많으셨습니다!

사실 Dependency Injection (Hilt 와 같은 라이브러리)를 도입하기 전에 레이어별로 의존성이 생기는건 현재 단계에서는 어쩔 수 없는 부분이 존재합니다. 현재 단계에서는 이러한 불편함이 존재하는구나 정도 생각하고 넘어가시면 될 것 같아요.

그리고 ViewModel이 사용자의 입력에 따라 사소한 값이라도 다 유기적으로 연결되어야 하는게 맞습니다. 추후에 two-way binding 이라고 해서 ViewModel과 View의 값이 쌍방향으로 연결되는 식으로 설계하는게 좋습니다. 그리고 View는 최대한 멍청하게 작성하셔야 합니다. View에서 표현되는 모든 데이터는 ViewModel에 있어야 하고 View는 단순히 보여주기 위함이라고 생각하시면 편하실거에요. 이 부분 또한 추후에 고도화 할 수 있기에 우선 현재단계에서는 넘어가셔도 좋을 것 같습니다 (충분히 잘 작성해주셨어요)

DB 부분도 현재 단계에서 오는 이러한 불편함을 한껏 만낏해주시면 됩니다 :)

마지막으로 Adapter 부분에 콜백이 많아짐에 따라서 추상화가 어렵다고 하셨는데 그러면 Adapter 내부에 Callback 관련 interface를 작성하시고 이것을 View (Fragment or Activity) 에서 구현한 후 해당 구현 메서드를 전달해주시는 방법으로 작성해보시는건 어떨까요?

점점 코드가 더 깔끔해지는게 보이네요. 디테일적인 부분이나 MVVM 관련된 내용들은 현재단계에서 많이 어려우실 수 있는데 잘 따라가고 계신것 같습니다. 앞으로 Keep Grinding 하시죠! 👍

Comment on lines +23 to +24
val newData = searchDb.queryAllSearchKeywords()
_keywords.postValue(newData)
Copy link

Choose a reason for hiding this comment

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

반복되는 동작같은데 메서드를 따로 빼도 괜찮을것 같습니다

Comment on lines +44 to +49
searchResultRecyclerView.addItemDecoration(
DividerItemDecoration(
activity,
DividerItemDecoration.VERTICAL
)
)
Copy link

Choose a reason for hiding this comment

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

ItemDecoration 활용하신거 훌륭합니다 👍


override fun getNewListSize(): Int = newList.size

override fun areItemsTheSame(oldItemPosition: Int, newItemPosition: Int): Boolean {
Copy link

Choose a reason for hiding this comment

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

보통 Diff.Callback을 작성하실 떄 areItemsTheSame 에서는 같은 아이템인지 확인을 보통 id와 같은걸로 진행하구요, areContentsTheSame에서 equals 비교를 합니다. 그래서 areItemsTheSame은 true지만 areContentsTheSame은 false의 경우 정확히 어떤 부분이 바뀌었는지를 찾아서 해당하는 부분만 다시 그릴 수 있습니다. 이를 payload로 전달하구요.
Diff 활용법은 추후에 다시 찾아보시고 적용해보심 좋을 것 같습니다 :)

}

class SearchResultAdapter(
private val inflater: LayoutInflater,
Copy link

Choose a reason for hiding this comment

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

inflater를 인자로 받지 말고 onCreateViewHolder에서 parent를 통해 생성해보세요!

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.

2 participants