전남대 Android_오진우_5주차 과제_Step1 #61
Conversation
There was a problem hiding this comment.
이번 step1 도 수고 많으셨어요 진우님!!
아직 hilt 를 사용하시는 것이 어색한 것 같아요. 모든 것은 SingletonComponent 로 제공될 필요는 없고, 한 화면에서만 사용될 예정이라면 ViewModelComponent 나 ActivityComponent 같은 것도 사용해보시는 것이 좋습니다!
또한 hilt 는 아키텍쳐에 요소를 많이 적용할수록 더욱 더 빛을 볼 수 있어요. 저번 과제에서 리뷰했던 것처럼 안드로이드에서 권장하는 아키텍쳐를 조금 더 적용해보시면 더더욱 좋을 것 같습니다 :)
또한 경합상태에 대한 테스트코드는 확실히 어렵긴 합니다만, hilt github 에서 race condition on injection 과 같이 검색해보시면 지원을 하는지 안하는지 알아보실 수 있을거에요
| @Singleton | ||
| @Provides | ||
| fun provideDataStore(@ApplicationContext context: Context) : DataStore<Preferences>{ | ||
| return context.dataStore | ||
| } |
There was a problem hiding this comment.
DatabaseModule 이라고 정의했기 때문에 DataStore 는 따로 빼두시는 편이 좋죠. 다만 LocalDataSourceModule 이라고 정의한다면, 함께 있어도 괜찮습니다!
| companion object { | ||
| @Volatile | ||
| private var instance: SavedPlaceDatabase? = null | ||
|
|
||
| fun getInstance(context: Context): SavedPlaceDatabase { | ||
| return instance ?: synchronized(this) { | ||
| Room.databaseBuilder( | ||
| context, SavedPlaceDatabase::class.java, PlaceContract.DATABASE_NAME | ||
| ).build().also { instance = it } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
여기에서 정의하지 마시고, hilt 의 Provides 내부 구현으로 옮겨주시는 것이 좋을 것 같습니다!
There was a problem hiding this comment.
hilt 를 사용하고 singleton 으로 제공한다면, 경합상태에 대한 고민을 크게 하지 않으셔도 됩니닷
|
|
||
| class SharedPreferenceRepository(private val dataStore: DataStore<Preferences>) { | ||
| @Singleton | ||
| class SharedPreferenceRepository @Inject constructor(private val dataStore: DataStore<Preferences>) { |
There was a problem hiding this comment.
SharedPreferences 는 Data Source 의 개념이기 떄문에 이부분을 Repository 로 정의하기보다는 PlaceRepository 나 SavedPlaceRepository 로 로직을 옮기는 것이 어떨까요?
| lateinit var map: MapView | ||
| lateinit var inputField: EditText | ||
| lateinit var searchIcon: ImageView | ||
| lateinit var errorTextView: TextView | ||
| lateinit var kakaoMap : KakaoMap | ||
| lateinit var resultLauncher : ActivityResultLauncher<Intent> | ||
| lateinit var sharedPreferencesRepository: SharedPreferenceRepository | ||
| lateinit var bottomSheetLayout : ConstraintLayout | ||
| lateinit var placeNameField : TextView | ||
| lateinit var placeLocationField : TextView | ||
| lateinit var bottomSheetBehavior : BottomSheetBehavior<ConstraintLayout> |
There was a problem hiding this comment.
위에 viewbinding, databinding 의존성도 있던데, 이부분도 리팩토링 해보시면 어떨까요?
| fun getSavedPlace() { | ||
| _savedPlace.value = (savedPlaceRepository.getAllSavedPlace()) | ||
| viewModelScope.launch(Dispatchers.IO) { |
There was a problem hiding this comment.
Dispatcher 도 주입받을 수 있도록 처리해주시는 것이 더욱 더 좋습니다!
https://developer.android.com/kotlin/coroutines/coroutines-best-practices
| fun getSavedPlace() { | ||
| _savedPlace.value = (savedPlaceRepository.getAllSavedPlace()) | ||
| viewModelScope.launch(Dispatchers.IO) { | ||
| _savedPlace.postValue(savedPlaceRepository.getAllSavedPlace()) |
There was a problem hiding this comment.
coroutine 을 사용하시는김에, 이부분도 StateFlow 로 변경하면 어때요?
https://developer.android.com/kotlin/flow/stateflow-and-sharedflow
오늘도 코드리뷰 부탁드립니다!
코드 작성하면서 어려웠던 점 & 느낀 점
멘토님께서 중점적으로 리뷰해주셨으면 하는 부분
추가로 궁금한 점
언제나 감사드립니다!