전남대 Android_오진우_2주차 과제_Step1 #25
Conversation
There was a problem hiding this comment.
MVVM 패턴을 대강은 알고있었지만, 실제 적용할 때 어떤식으로 책임을 분배해야할지 감이 잘 안잡혔습니다.
제 코드상에서는 DBHelper 클래스가 직접 데이터베이스를 접근하고, Repository 클래스에서 결과를 받아와 가공해서, ViewModel에서 사용하는 식으로 구성하였습니다.
예를 들어 SQLiteOpenHelper 클래스를 상속받은 DBHelper 클래스에서 readData()라는 메서드를 만들었습니다.
이 메서드를 통해 데이터베이스에서 장소 테이블의 모든 데이터를 읽어올 수 있게끔 하였습니다.
이런 메서드가 DBHelper 클래스 내에서 위치해야 할지, 아니면 Repository 클래스 내에서 위치해야 할지, 아니면 DAO 클래스를 하나 더 만들어서 처리해야 할 지, 감이 잘 안잡혔던 것 같습니다.
DAO 클래스를 하나 더 만들어서 처리하는 것이 조금 더 좋습니다. 추가적으로 Room 에서는 어떻게 처리하고 있는지 살펴보는 것이 좋겠네요! (이참에 Room 으로 변경해주셔도 좋구요!)
MVVM 패턴이 역할에 맞게끔 잘 적용되었는 지 리뷰해주셨으면 합니다.
현재는 MVVM 패턴대로 진행되었다고 보기는 어렵습니다. MVVM 이라기 보다는 DB 로직을 따로 분리한 정도에 그치는 것 같아요. 전체적으로 맥락상 여기에 UiState 를 통해 Activity 와 ViewModel 이 소통한다면 좋을 것 같네요.
만약 다른 데이터베이스를 추가로 만들어서 사용한다고 할때 새로운 DBHelper, Repository 클래스를 만들어줘야 하는지 궁금합니다!
이런 경우 Repository 를 새로 만들기 보다는 새로운 DBHelper 를 만드는 것이 좋을 것 같아요. DB 와 DB 가 조합해서 같은 결과값을 내보낼 때면 같은 Repository 를 사용하면 좋을 것 같고, 아예 용도가 다르다면 다른 Repository 를 사용하는 것이 조금 더 좋을 것 같습니다.
| object PlaceContract{ | ||
| object PlaceEntry : BaseColumns { | ||
| const val TABLE_NAME = "PLACE" | ||
| const val COLUMN_NAME = "NAME" | ||
| const val COLUMN_LOCATION = "LOCATION" | ||
| const val COLUMN_CATEGORY = "CATEGORY" | ||
| } |
| import android.database.sqlite.SQLiteOpenHelper | ||
|
|
||
|
|
||
| class PlaceDBHelper(context:Context) : SQLiteOpenHelper(context, "place.db", null, 1){ |
There was a problem hiding this comment.
place.db 와 같은 것들도 const 로 정의하면 조금 더 좋겠네요!
|
|
||
| override fun onUpgrade(db: SQLiteDatabase?, oldVersion: Int, newVersion: Int) { | ||
| db?.execSQL("DROP TABLE IF EXISTS ${PlaceContract.PlaceEntry.TABLE_NAME};") | ||
| onCreate(db) |
There was a problem hiding this comment.
onCreate 를 직접 호출하기 보다 onCreate 에서 수행하는 로직을 따로 메소드로 분리하는 것은 어떨까요?
override fun onCreate(db: SQLiteDatabase) {
createLocationTable(db)
}
override fun onUpgrade(db: SQLiteDatabase, oldVersion: Int, newVersion) {
db?.execSQL("DROP TABLE IF EXISTS ${PlaceContract.PlaceEntry.TABLE_NAME};")
createLocationTable(db)
}
private fun createLocationTable(db: SQLiteDatabase) {
db?.execSQL("CREATE TABLE " +
"${PlaceContract.PlaceEntry.TABLE_NAME}( " +
" ${PlaceContract.PlaceEntry.COLUMN_NAME} varchar(60) not null ," +
" ${PlaceContract.PlaceEntry.COLUMN_LOCATION} varchar(255) not null, " +
" ${PlaceContract.PlaceEntry.COLUMN_CATEGORY} varchar(30) );"
)
}| fun insertDummyData(){ | ||
| val name = "카페" | ||
| val category = "카페" | ||
| val location = "서울 성동구 성수동" | ||
| for(i in 1..10){ | ||
| insertData(name + i, location + i, category) | ||
| } | ||
| readData() | ||
| } |
| import campus.tech.kakao.map.model.Place | ||
|
|
||
| class Repository (val dbHelper: PlaceDBHelper){ | ||
| fun getAllPlace() : MutableList<Place>{ |
There was a problem hiding this comment.
getAllPlace 는 수정될 여지가 없기 때문에 MutableList 를 반환하기 보다는 Immutable 한 List 를 반환하는 것이 어떨까요?
| fun getAllPlace() : MutableList<Place>{ | |
| fun getAllPlace() : List<Place>{ |
| import campus.tech.kakao.map.db.PlaceDBHelper | ||
| import campus.tech.kakao.map.model.Place | ||
|
|
||
| class Repository (val dbHelper: PlaceDBHelper){ |
There was a problem hiding this comment.
또한 Repository 가 PlaceDBHelper 를 가지고 있는데, PlaceDBHelper 의 close 를 호출하는 부분이 없네요! 앱 사용에 따라 반드시 close 되어야 합니다. 앱 라이프사이클에 따라 close 시점을 고민해보시면 좋을 것 같습니다!
안녕하세요 멘토님 :)
코드리뷰 부탁드립니다!
코드 작성하면서 어려웠던 점
코드 리뷰 시 멘토님이 중점적으로 리뷰해주셨으면 하는 부분
추가 질문
언제나 감사드립니다!