-
Notifications
You must be signed in to change notification settings - Fork 32
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 박정훈 4주차 과제 Step2 #93
base: pjhn
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4주차 과제도 고생많으셨습니다!
|
||
class PlaceRepositoryImpl(context: Context): | ||
SQLiteOpenHelper(context, PlaceContract.DATABASE_NAME, null, 1), | ||
PlaceRepository { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
장소와 관련된 데이터를 담당하는 Repository 이니, Repository가 SQLiteOpenHelper를 상속받기 보다는 생성자를 통해 open helper 객체를 전달받는 것이 조금더 좋을것 같습니다. 내부에서는 local, remote에 따라서 알맞은 객체로 연결을 해주는 형태가 되는거죠.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
확인했습니다! 수정하도록 하겠습니다
private fun initMap() { | ||
val sharedPreferences = getSharedPreferences("LastVisitedPlace", MODE_PRIVATE) | ||
val placeName = sharedPreferences.getString("placeName", null) | ||
val roadAddressName = sharedPreferences.getString("roadAddressName", null) | ||
val categoryName = sharedPreferences.getString("categoryName", null) | ||
val yPos = sharedPreferences.getString("yPos", null) | ||
val xPos = sharedPreferences.getString("xPos", null) | ||
|
||
if (placeName != null && roadAddressName != null && categoryName != null && yPos != null && xPos != null) { | ||
val place = Place("", placeName, roadAddressName, categoryName, xPos, yPos) | ||
updateMapWithPlaceData(place) | ||
showBottomSheet(place) | ||
} | ||
} | ||
|
||
private fun saveLastVisitedPlace(place: Place) { | ||
val sharedPreferences = getSharedPreferences("LastVisitedPlace", MODE_PRIVATE) | ||
val editor = sharedPreferences.edit() | ||
editor.putString("placeName", place.place) | ||
editor.putString("roadAddressName", place.address) | ||
editor.putString("categoryName", place.category) | ||
editor.putString("yPos", place.yPos) | ||
editor.putString("xPos", place.xPos) | ||
editor.apply() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sharedPreference에 저장하는것도 기기에 데이터를 저장한다는 관점에서는 data layer에서 수행하는게 어떨까 합니다. UI layer에서는 저장된 place 정보만 전달받아 지도에 보여주는 형태이고, 저장된 place를 가져오는 행위는 ViewModel에서 Repository를 통해서 수행하는것이 조금더 적절할것 같습니다
private val _places = searchText.asFlow() | ||
.debounce(500L) | ||
.flatMapLatest { query -> | ||
if (query.isNotBlank()) { | ||
liveData { | ||
updateTempPlaces(query) | ||
delay(300L) | ||
emit(getAllLiveDataPlaces()) | ||
}.asFlow() | ||
} else { | ||
flowOf(emptyList<Place>()) | ||
} | ||
} | ||
.asLiveData() | ||
val places: LiveData<List<Place>> get() = _places |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UI로 데이터를 전달해주기 위한 수단으로도 flow를 사용할 수 있으니, LiveData로 변환할 필요는 없을듯합니다. 이참에 flow를 사용해보는것도 도전해보시죠!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵 flow 사용하는 것으로 코드 수정했습니다!
private fun updateTempPlaces(keyword: String, page: Int = 1): List<Place>{ | ||
val tempPlaces = mutableListOf<Place>() | ||
val MAX_PAGE = 3 | ||
val SIZE = 15 | ||
for (i in page..MAX_PAGE) { | ||
kakaoApiLauncher.getPlacesBySearchText( | ||
BuildConfig.KAKAO_REST_API_KEY, keyword,SIZE, i, | ||
onSuccess = { places -> | ||
tempPlaces.addAll(places) | ||
if (i == MAX_PAGE) { | ||
updatePlaces(tempPlaces) | ||
tempPlaces.clear() | ||
} | ||
}, | ||
onError = { errorMessage -> | ||
Toast.makeText(getApplication(), errorMessage, Toast.LENGTH_SHORT).show() | ||
} | ||
) | ||
} | ||
return tempPlaces | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
카카오 api를 통해 장소를 조회하는것도 data network layer에서 수행되는게 조금더 좋을것 같습니다.
AAC ViewModel은 UI layer로 볼수있으므로, 여기서는 조회해온 place데이터만 넘겨받는 형태가 어떨까요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵 확인했습니다.
private fun showErrorPage(error: Exception){ | ||
setContentView(R.layout.error_page) | ||
tvErrorMessage = findViewById(R.id.tvErrorMessage) | ||
tvErrorMessage.text = "지도 인증에 실패했습니다.\n다시 시도해주세요.\n"+ error.message | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
에러 페이지를 보여주고 싶을때 Activity의 setContentView
를 다시 호출하기 보다는 XML 위 선언된 View를 visible/gone 처리만 해주는것이 좋을것 같습니다. 지금은 네트워크 연결이 끊어졌을때는 에러페이지가 보이지만, 다시 연결되었을때 정상화면이 보이는처리도 필요해보이네요
- 패키지명 변경: view -> presentation - 파일명 변경 - ViewActivity.kt-> SearchActivity.kt - PlaceViewModel.kt -> SearchViewModel.kt
테스트 코드 작성
코드 작성하면서 어려웠던 점
코드 리뷰 시, 멘토님이 중점적으로 리뷰해줬으면 하는 부분
라이브러리 충돌 에러는 최대한 빠른 시일 내로 해결하도록 하겠습니다.