-
Notifications
You must be signed in to change notification settings - Fork 29
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 박정훈 6주차 과제 Step2 #72
base: pjhn
Are you sure you want to change the base?
Conversation
- 호출 성공, 실패 시에 따라 결과 화면 바뀌는 기능 - 성공 시, 서비스 상태 확인 - 실패 시, 네트워크 상태 확인
# Conflicts: # README.md # app/src/main/AndroidManifest.xml # app/src/main/java/campus/tech/kakao/map/di/NetworkModule.kt # app/src/main/java/campus/tech/kakao/map/di/ViewModelModule.kt # app/src/main/res/drawable/icon_cancel.xml # app/src/main/res/drawable/icon_location3.png # app/src/main/res/drawable/icon_search.xml # app/src/main/res/drawable/searchview_background2.xml # app/src/main/res/layout/activity_main.xml # app/src/main/res/layout/bottom_sheet.xml # app/src/main/res/layout/list_item.xml # app/src/main/res/layout/log_item.xml # app/src/main/res/values/themes.xml
- module에서 firebase 객체 세팅 후 주입 - repository 싱글톤 적용 - 스플래시 화면 viewmodel 초기값 설정
- isNetworkActive() 에 application context 주입하는 방식으로 변경 - LastVisitedPlaceManger에 Sharedpreferences 객체를 주입하는 방식으로 변경 - SearchViewModel에서 지정한 페이지 만큼 api 호출 - data class 멤버 val 선언 - 리사이클러 뷰의 어댑터는 by lazy 방식으로 변경 - 리스너 모듈 생성 - SearchViewAcitivty에서 사용되는 리스너 인터페이스 생성
# Conflicts: # app/src/main/java/campus/tech/kakao/map/data/RemoteConfigRepository.kt # app/src/main/java/campus/tech/kakao/map/di/NetworkModule.kt # app/src/main/java/campus/tech/kakao/map/presentation/splash/SplashScreenActivity.kt # app/src/main/java/campus/tech/kakao/map/presentation/splash/SplashScreenViewModel.kt
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.
@Pjhn Good 👍 5주차 코멘트도 모두 반영해주셨고, 6주차의 RemoteConfig / FCM 커리큘럼도 모두 적용하셨네요. 코멘트 확인한번 부탁드립니다. 머지는 일요일 저녁 혹은 월요일에 하도록 할게요.
5주차 피드백에서 말씀해주신 저장소의 getPlaces()를 3번 호출하는 코드는
viewModel 단에서 처리해야 하는지 useCase가 있다면 그 쪽에서 처리해도 되는지 궁금합니다
UseCase는 장소 데이터를 가져오는 동작
을 다른곳에서 재활용 하거나, ViewModel 에서 분리해서 관리하고 싶을때 만들면 효율적인것 같습니다. 3번의 api 호출이 완료된 List 를 리턴하는 동작을 메소드로 만들어도 되겠지만, 그 동작만을 수행하는 클래스로 역할분리 시키는거죠. 지금은 ViewModel에서 PlaceRepository를 3번 호출해서 관리하고 있지만, UseCase를 만들게 되면, UseCase만 호출하면 되기 때문에 좀더 깔끔하게 작성할 수 있게 될것 같네요
companion object { | ||
@Volatile | ||
private lateinit var appInstance: PlaceApplication | ||
fun isNetworkActive(): Boolean { | ||
fun isNetworkActive(@ApplicationContext context: Context): Boolean { |
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.
static function 으로 선언되어있고, hilt를 통해 context가 전달되는 방식이 아니기 때문에 @ApplicationContext
어노테이션은 제거해도 괜찮을것 같습니다
@Provides | ||
@ActivityScoped | ||
fun provideSearchActivityRecyclerviewListener(activity: Activity) | ||
: SearchActivityRecyclerviewListener { | ||
val searchActivity = activity as SearchActivity | ||
|
||
return object : SearchActivityRecyclerviewListener { | ||
override fun onPlaceClick(place: Place) { | ||
searchActivity.handlePlaceClick(place) | ||
} | ||
|
||
override fun onLogDelBtnClick(logId: String) { | ||
searchActivity.handleLogDelBtnClick(logId) | ||
} | ||
} | ||
} |
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내부에서 사용하는 Listener까지 Hilt를 통해 주입받기 보다는 Activity내부에서 생성하는편입니다.
범용적으로 사용할 수 있는 클래스들 위주로만 제공해도 괜찮을것 같고, 혹시나 activity의 context가 필요한 상황이라면 @ActivityContext
를 붙여서 제공받는게 어떨까 합니다. (참고)
만약, 지금 SearchActivity 가 아닌 다른곳에서 주입받게 되면 해당 activity as SearchActivity
부분에서 에러가 발생하게 될것 같습니다.
val response = kakaoApi.getSearchKeyword( | ||
key = BuildConfig.KAKAO_REST_API_KEY, | ||
query = placeName, | ||
size = 15, | ||
page = page | ||
) |
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.
한번에 45개 정도로 많은 데이터를 가져오고 싶은 상황이라면, size를 page * 3 정도로 하거나 loadSize를 인자로 넘겨받는 방법도 괜찮을것 같습니다. 현재는 page 1~3 을 모두 호출해야하니 api 통신이 3회 이뤄지게 되는것이니, api 통신횟수를 줄이면서 원하는 양의 데이터를 가져오게 할 수 있을것 같네요
if (ContextCompat.checkSelfPermission( | ||
this, | ||
android.Manifest.permission.POST_NOTIFICATIONS | ||
) == | ||
PackageManager.PERMISSION_GRANTED | ||
) { | ||
// FCM SDK (and your app) can post notifications. | ||
} else if (shouldShowRequestPermissionRationale(android.Manifest.permission.POST_NOTIFICATIONS)) { |
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.
권한 (퍼미션) 이 허용되어 있을때는 아무 동작을 하지 않으므로, true 인것을 확인하기 보다는 !=
으로 false 일때만 확인하도록 한다면 if-else문을 하나 줄일 수 있을것 같습니다
반영 사항 및 구현 기능
viewModel 단에서 처리해야 하는지 useCase가 있다면 그 쪽에서 처리해도 되는지 궁금합니다