Skip to content
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

[REFACTOR] 출발지 모드 세팅 / SearchResultEntity에서 mode 제거 및 리팩토링 #274

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,4 @@ data class SearchResultEntity(
val fullAddress: String,
val name: String,
val locationLatLng: LatLng?,
val mode: String
) : Parcelable
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ class DepartureSearchRepositoryImpl @Inject constructor(private val departureSou
fullAddress = makeMainAddress(it),
name = it.name ?: "",
locationLatLng = LatLng(it.noorLat.toDouble(), it.noorLon.toDouble()),
mode = "searchLocation" //현위치, 지도에서 출발과 구분하기 위한 식별자
)
}
return changedData
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,10 @@ import com.runnect.runnect.databinding.BottomsheetRequireCourseNameBinding
import com.runnect.runnect.presentation.MainActivity
import com.runnect.runnect.presentation.countdown.CountDownActivity
import com.runnect.runnect.presentation.login.LoginActivity
import com.runnect.runnect.presentation.search.SearchActivity.Companion.EXTRA_DEPARTURE_SET_MODE
import com.runnect.runnect.presentation.state.UiState
import com.runnect.runnect.util.multipart.ContentUriRequestBody
import com.runnect.runnect.util.extension.setActivityDialog
import com.runnect.runnect.util.multipart.ContentUriRequestBody
import dagger.hilt.android.AndroidEntryPoint
import kotlinx.android.synthetic.main.custom_dialog_make_course.view.btn_run
import kotlinx.android.synthetic.main.custom_dialog_make_course.view.btn_storage
Expand Down Expand Up @@ -77,6 +78,7 @@ class DrawActivity :
private lateinit var animDown: Animation
private lateinit var animUp: Animation
private lateinit var searchResult: SearchResultEntity
private lateinit var departureSetMode: String
private lateinit var captureUri: Uri

private val coords = mutableListOf<LatLng>()
Expand Down Expand Up @@ -108,7 +110,10 @@ class DrawActivity :

private fun getSearchIntent() {
searchResult =
intent.getParcelableExtra(EXTRA_SEARCH_RESULT) ?: throw Exception("데이터가 존재하지 않습니다.")
intent.getParcelableExtra(EXTRA_SEARCH_RESULT)
?: throw Exception("unknown-search-result")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

제가 pr 올린 코드에 아래 확장함수들 추가해뒀어요!

/** Retrieve parcelable extra data from the intent and support app compatibility */
inline fun <reified T : Parcelable> Intent.getCompatibleParcelableExtra(key: String): T? = when {
    SDK_INT >= TIRAMISU -> getParcelableExtra(key, T::class.java)
    else -> getParcelableExtra(key) as? T
}

inline fun <reified T : Serializable> Intent.getCompatibleSerializableExtra(key: String): T? = when {
    SDK_INT >= TIRAMISU -> getSerializableExtra(key, T::class.java)
    else -> getSerializableExtra(key) as? T
}

SDK 버전에 따라 다르게 대응해주는 게 좋을 거 같습니다!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

혹시 여기서 던진 Exception은 어디서 예외처리 해주는지 알 수 있을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

위에 가이드 주신 확장함수 둘 다 SDK_INT >= TIRAMISU로 돼있는데 sdk 버전에 따라 분기 처리가 된 것이 맞을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

예외 시 특정 처리를 해주는 코드는 작성하지 않았고 단순히 LogCat에서 로그 확인 목적으로만 저렇게 작성했습니다! 혹시 문제 되는 부분이 있을까요? (몰라서 물어보는 것)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

TIRAMISU 버전 이전인지 이후인지에 따라 작성하는 코드가 달라져야 하는데
이걸 매번 작성하는 것이 번거롭기 때문에 확장함수로 따로 만든 것입니다!

image

현재 pr 올린 코드는 버전에 따라 다른 코드를 작성해주지 않았기 때문에, 위와 같이 deprecated 된 함수를 호출하게 됩니다.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

throw로 강제 Exception을 던지게 되면 런타임 에러가 발생하지 않을까 싶은데요!
여기서 던진 예외를 try-catch문이나 runCatching 등으로 받아서 따로 처리해주는 로직이 필요할 거 같습니다!

departureSetMode = intent.getStringExtra(EXTRA_DEPARTURE_SET_MODE)
?: throw Exception("unknown-departure-set-mode")
}

private fun initMapView() {
Expand All @@ -135,17 +140,19 @@ class DrawActivity :
setLocationChangedListener()
setCameraFinishedListener()
}

private fun initMode() {
when (searchResult.mode) {
when (departureSetMode) {
"searchLocation" -> initSearchLocationMode()
"currentLocation" -> initCurrentLocationMode()
"customLocation" -> initCustomLocationMode()
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

현재 디벨롭에 머지된 코드와 충돌되는 부분인 거 같아요! 충돌 해결하고 다시 푸시 부탁드립니다 :)

}

private fun initSearchLocationMode() {
isSearchLocationMode = true

with(binding){
with(binding) {
tvGuide.isVisible = false
btnDraw.text = CREATE_COURSE
}
Expand All @@ -168,6 +175,7 @@ class DrawActivity :
}
}
}

private fun initCurrentLocationMode() {
isCurrentLocationMode = true
isMarkerAvailable = true
Expand All @@ -180,6 +188,7 @@ class DrawActivity :
hideDeparture()
showDrawCourse()
}

private fun initCustomLocationMode() {
isCustomLocationMode = true

Expand Down Expand Up @@ -245,6 +254,7 @@ class DrawActivity :
val cameraPosition = naverMap.cameraPosition
return cameraPosition.target // 중심 좌표
}

private fun setLocationChangedListener() {
naverMap.addOnLocationChangeListener { location ->
currentLocation = LatLng(location.latitude, location.longitude)
Expand All @@ -256,7 +266,7 @@ class DrawActivity :
//같은 scope 안에 넣었으니 setDepartureLatLng 다음에 drawCourse가 실행되는 것이 보장됨
//이때 isFirstInit의 초기값을 true로 줘서 최초 1회는 실행되게 하고 이후 drawCourse 내에서 isFirstInit 값을 false로 바꿔줌
//뒤의 조건을 안 달아주면 다른 mode에서는 버튼을 클릭하기도 전에 drawCourse()가 돌 거라 안 됨.
if(isFirstInit && isCurrentLocationMode){
if (isFirstInit && isCurrentLocationMode) {
drawCourse(departureLatLng = departureLatLng)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,15 +81,6 @@ class SearchActivity :
binding.recyclerViewSearch.adapter = searchAdapter
}

override fun selectItem(item: SearchResultEntity) {
startActivity(
Intent(this, DrawActivity::class.java).apply {
addFlags(Intent.FLAG_ACTIVITY_NO_ANIMATION)
putExtra(EXTRA_SEARCH_RESULT, item) //mode == "searchLocation"
}
)
}

private fun showEmptyView() {
with(binding) {
ivNoSearchResult.isVisible = true
Expand Down Expand Up @@ -187,28 +178,28 @@ class SearchActivity :
}
}

fun startCurrentLocation() {
private fun setDeparture(mode: String, item: SearchResultEntity) {
startActivity(
Intent(this, DrawActivity::class.java).apply {
addFlags(Intent.FLAG_ACTIVITY_NO_ANIMATION)
putExtra(
EXTRA_SEARCH_RESULT,
SearchResultEntity(fullAddress = "", name = "", locationLatLng = null, mode = "currentLocation")
)
putExtra(EXTRA_SEARCH_RESULT, item)
putExtra(EXTRA_DEPARTURE_SET_MODE, mode)
}
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DrawActivity로 화면 전환이 이루어지는 코드이므로

navigateToCourseDrawScreen 또는 navigateToCourseDrawScreenWinthBundle 이런 함수명은 어떨지 제안해봅니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

좋습니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

반영했습니다!

}

fun startCustomLocation() {
startActivity(
Intent(this, DrawActivity::class.java).apply {
addFlags(Intent.FLAG_ACTIVITY_NO_ANIMATION)
putExtra(
EXTRA_SEARCH_RESULT,
SearchResultEntity(fullAddress = "", name = "", locationLatLng = null, mode = "customLocation")
)
}
)
override fun startSearchLocation(item: SearchResultEntity) {
setDeparture("searchLocation", item)
}

private fun startCurrentLocation() {
val emptyItem = SearchResultEntity(fullAddress = "", name = "", locationLatLng = null)
setDeparture("currentLocation", emptyItem)
}

private fun startCustomLocation() {
val emptyItem = SearchResultEntity(fullAddress = "", name = "", locationLatLng = null)
setDeparture("customLocation", emptyItem)
Copy link
Member

@leeeha leeeha Nov 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

현위치에서 시작하는 currentLocation 모드, 출발지를 지도 상에서 선택하는 customLocation 모드에서도
SearchResultEntity 데이터 클래스를 넘겨줘야 하는 이유가 있을까요??

private fun navigateToCourseDrawScreen(mode: String, searchResult: SearchResultEntity? = null) {
      startActivity(
          Intent(this, DrawActivity::class.java).apply {
              addFlags(Intent.FLAG_ACTIVITY_NO_ANIMATION)
              putExtra(EXTRA_DEPARTURE_SET_MODE, mode) 
              putExtra(EXTRA_SEARCH_RESULT, searchResult) 
          }
      )
}

// 검색 모드 
navigateToCourseDrawScreen(mode = DepartureMode.SEARCH, searchResult = SearchResultEntity)

// 현위치 모드 
navigateToCourseDrawScreen(mode = DepartureMode.CURRENT)

// 커스텀 위치 모드 
navigateToCourseDrawScreen(mode = DepartureMode.CUSTOM)

이런 식으로 현위치 모드, 커스텀 위치 모드에서는 default arguments 이용해서 SearchResultEntity를 null로 설정하는 게 어떨지 제안해봅니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

하나의 함수로 묶어서 중복 코드를 줄이는 것에만 신경을 썼었는데 말씀해주신 방향이 깔끔하겠네요. 반영하겠습니다! 땡큐!

}

//키보드 밖 터치 시, 키보드 내림
Expand All @@ -228,6 +219,7 @@ class SearchActivity :

companion object {
const val EXTRA_SEARCH_RESULT = "searchResult"
const val EXTRA_DEPARTURE_SET_MODE = "departureSetMode"
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,9 @@ class SearchAdapter(private val searchClickListener: OnSearchClick) :

fun bindData(data: SearchResultEntity) {
binding.searchResultEntity = data
} //여기도 dto 구독해서 ui 바인딩 하는 목적이라 낭비임.

fun bindViews(data: SearchResultEntity) {
binding.root.setOnClickListener {
searchClickListener.selectItem(data) //data는 넣었고 구체적인 동작은 오버라이드할 때
searchClickListener.startSearchLocation(data)
}
}

Expand All @@ -35,7 +33,6 @@ class SearchAdapter(private val searchClickListener: OnSearchClick) :

override fun onBindViewHolder(holder: SearchResultItemViewHolder, position: Int) {
holder.bindData(currentList[position])
holder.bindViews(currentList[position])
}

class Differ : DiffUtil.ItemCallback<SearchResultEntity>() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@ package com.runnect.runnect.util.callback
import com.runnect.runnect.data.dto.SearchResultEntity

interface OnSearchClick {
fun selectItem(item: SearchResultEntity)
fun startSearchLocation(item: SearchResultEntity)
}
Loading