경북대 Android 남지연 4주차 과제 Step1 #56
Conversation
| private lateinit var locationSearcher: LocationSearcher | ||
| private lateinit var keywordViewModel: KeywordViewModel | ||
|
|
||
| companion object { |
There was a problem hiding this comment.
Kotlin convention에서 companio object의 위치는 맨 아래로 지시하고 있습니다. 최하단으로 내리는게 좋겠네요.
|
|
||
| override fun onCreate(savedInstanceState: Bundle?) { | ||
| super.onCreate(savedInstanceState) | ||
| setContentView(R.layout.activity_main) | ||
|
|
||
| // 카카오 지도 초기화 | ||
| locationSearcher = LocationSearcher(this) |
There was a problem hiding this comment.
LocationSearcher의 getInstance는 언제 쓰는걸까요?
| ) { result -> | ||
| if (result.resultCode == RESULT_OK) { | ||
| val data = result.data | ||
| data?.let { |
There was a problem hiding this comment.
에러처리할 때 고민해봐야 할 부분은 데이터를 받지 못한 경우(이 경우에는 검색 데이터를 못 받아온 경우겠네요) 어떤 방식으로 UX를 보장해야 할까?를 고민해 보셔야 할 것 같네요. 현재의 경우라면 (0,0) 포지션으로 이동할 것 같은데 우아하진 않은것 같아요. 이런 경우 Toast Message를 간략히 보여주거나 하는 방식을 사용해 볼 수 있겠네요.
그리고 저는 data?.let 과 같은 scope function은 최대한 짧게 작성하려고 해요. 지금 경우는 매우 길어진 것 같으니 null일 때에 대한 처리를 위에서 하고 depth를 줄이는게 가독성엔 더 좋겠네요.
| // 인증 후 API가 정상적으로 실행될 때 호출됨 | ||
| override fun onMapReady(map: KakaoMap) { | ||
| kakaoMap = map | ||
| labelLayer = kakaoMap.labelManager?.layer!! |
There was a problem hiding this comment.
!! 사용은 지양하셔야 합니다.
안드로이드 내에서 이유없는 에러는 정말 빈번합니다. 안드로이드에서 !!는 단숨에 fatal error를 만드는 지뢰같은 표현식입니다. 행여나 그럴 수 있지~ 라는 마인드로 대처하셔야 합니다.
| // 에러 화면 초기화 | ||
| errorLayout = findViewById(R.id.error_layout) | ||
| errorMessage = findViewById(R.id.error_message) | ||
| errorDetails = findViewById(R.id.error_details) | ||
| retryButton = findViewById(R.id.retry_button) | ||
| retryButton.setOnClickListener { onRetryButtonClick() } |
| } | ||
|
|
||
| private fun addLabel(placeName: String?, roadAddressName: String?, latitude: Double, longitude: Double) { | ||
| if (placeName != null && roadAddressName != null) { |
There was a problem hiding this comment.
이런 if 문도 줄일 수 있어요
| if (placeName != null && roadAddressName != null) { | |
| if (placeName == null || roadAddressName == null) return |
| private lateinit var errorDetails: TextView | ||
| private lateinit var retryButton: ImageButton | ||
| private lateinit var kakaoMap: KakaoMap | ||
| private lateinit var labelLayer: LabelLayer |
| val sharedPreferences = getSharedPreferences(PREFS_NAME, Context.MODE_PRIVATE) | ||
| with(sharedPreferences.edit()) { | ||
| putFloat(PREF_LATITUDE, latitude.toFloat()) | ||
| putFloat(PREF_LONGITUDE, longitude.toFloat()) | ||
| putString(PREF_PLACE_NAME, placeName) | ||
| putString(PREF_ROAD_ADDRESS_NAME, roadAddressName) | ||
| apply() | ||
| } |
There was a problem hiding this comment.
SharedPreferences 조작은 View의 역할은 아닙니다. ViewModel이나 Repository 쪽으로 빼는게 좋겠네요
| val sharedPreferences = getSharedPreferences(PREFS_NAME, Context.MODE_PRIVATE) | ||
| if (sharedPreferences.contains(PREF_LATITUDE) && sharedPreferences.contains(PREF_LONGITUDE)) { | ||
| val latitude = sharedPreferences.getFloat(PREF_LATITUDE, 0.0f).toDouble() | ||
| val longitude = sharedPreferences.getFloat(PREF_LONGITUDE, 0.0f).toDouble() | ||
| val placeName = sharedPreferences.getString(PREF_PLACE_NAME, "") ?: "" | ||
| val roadAddressName = sharedPreferences.getString(PREF_ROAD_ADDRESS_NAME, "") ?: "" | ||
|
|
||
| if (placeName.isNotEmpty() && roadAddressName.isNotEmpty()) { | ||
| Log.d(TAG, "Loaded last marker position: lat=$latitude, lon=$longitude, placeName=$placeName, roadAddressName=$roadAddressName") | ||
| addLabel(placeName, roadAddressName, latitude, longitude) | ||
| val position = LatLng.from(latitude, longitude) | ||
| moveCamera(position) | ||
| updateBottomSheet(placeName, roadAddressName) | ||
| } else { | ||
| Log.d(TAG, "No place name or road address name found") | ||
| } | ||
| } else { | ||
| Log.d(TAG, "No last marker position found in SharedPreferences") | ||
| } |
There was a problem hiding this comment.
MainActivity의 역할을 다하기 위해 SharedPreferences에 값을 저장하는걸 아는것까지가 MainActivity의 책임일지 생각해보시면 이 로직을 어디에 작성해야 할지 경계가 분명해질거에요
There was a problem hiding this comment.
SharedPreferences를 View 내에서 직접 로드해서 View에 표현하는게 View의 역할이 아닙니다.
View가 필요한 데이터를 ViewModel에서 내려받아 단순히 표현! 하는게 View의 역할입니다.
| val retrofit = Retrofit.Builder() | ||
| .baseUrl(BuildConfig.KAKAO_BASE_URL) | ||
| .addConverterFactory(GsonConverterFactory.create()) | ||
| .build() | ||
| val api = retrofit.create(KakaoLocalApi::class.java) | ||
| val api = MyApplication.retrofit.create(KakaoLocalApi::class.java) |
There was a problem hiding this comment.
Retrofit 생성을 외부로 빼신건 좋은데 아직 api에 대한 인스턴스 취득을 Activity에서 해주고 있네요.
자 그럼 이제 어떻게 해야 할까요? api 사용부는 ViewModel로 옮겼고 인스턴스 생성도 외부로 옮겼는데 아직도 Retrofit 인스턴스에 대한 접근이 필요하다면 의존성이 생기는건데 이를 피할 수 있는 방법이 뭐가 있을까요?
고민 한번 해보심 좋을 것 같네요. 아마 다음 주차에서 배우실 것 같아요 👍
과제 수행시 느꼈던 어려움 & 느낀점
Logcat으로 오류를 잡는 게 어려웠다.import된 게 많아서 코드에서 수정이 필요한 부분을 확인하는 것이 어려웠다.API 문서를 보는 게 아직 많이 서툴다는 것을 느꼈다.label을 사용해서 마커표시를 했다는 걸 들어서 그나마 수월하게 과제를 수행할 수 있었다.commit을 하는 것을 또 놓쳐서 다음 과제 수행 시에는 이 부분을 신경써야겠다고 느꼈다.중점적으로 리뷰해주셨으면 하는 부분
MainActivity가 엄청 길어졌는데, 수정이 필요한 건지 궁금합니다.bottomsheet를 제대로 사용한 게 맞는지 궁금합니다 !step0리뷰에서RelativeLayout이 아닌 다른 레이아웃을 사용해보라고 하셨는데, Logcat으로 오류를 확인하며CoordinatorLayout로 수정하였습니다. 이와 같은 방식으로 수정한 것이 맞을까요?onMapError()호출 시 에러 화면을 보여준다는 걸 제가 맞게 이해하고 구현한 것이 맞을까요?Step0리뷰에서MyAppication에서 Retrofit을 싱글톤으로 만들어보라고 하신 부분을 제가 맞게 반영한 건지 확인 부탁드립니다 !