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

Conversation

unam98
Copy link
Contributor

@unam98 unam98 commented Nov 7, 2023

📌 개요

closed #258

매직 리터럴은 #272 PR merge 후 Enum Class를 활용하여 제거해줄 예정입니다!

✨ 작업 내용

  • SearchResultEntity라는 이름으로 mode가 묶여서 하나로 관리되는 게 부적절하다는 판단이 들어서 제거해주었고 putExtra에 서로 다른 key 값으로 넘기는 방향으로 수정
  • startActivtiy 관련 중복 코드 제거

✨ PR 포인트

  • data class 이름 지을 때 어떤 분들은 Dto로, 어떤 분들은 Entity라는 이름을 다시는데 어떤 경우에 이렇게 이름을 구분 짓는지 아시나요?! (몰라서 물어보는 것)

📸 스크린샷/동영상

@unam98 unam98 added 우남 🐼 우남 담당 REFACTOR 🧹 코드 리팩토링 MOD ☁ 코드 및 내부 파일 수정 labels Nov 7, 2023
@unam98 unam98 requested a review from leeeha November 7, 2023 00:59
@unam98 unam98 self-assigned this Nov 7, 2023
@leeeha
Copy link
Member

leeeha commented Nov 9, 2023

위의 글들 외에도 DAO, DTO, VO, Entity의 차이점이 무엇인지 정리한 글이 많더라구요! 참고하면 좋을 거 같습니다!
저도 DTO와 Entity의 차이점이 무엇인지 잘 모르겠어서 덕분에 알아보게 됐네요!

Copy link
Member

@leeeha leeeha left a comment

Choose a reason for hiding this comment

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

디벨롭 브랜치와의 충돌 해결 부탁드립니다! :)

@@ -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 등으로 받아서 따로 처리해주는 로직이 필요할 거 같습니다!

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.

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

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.

반영했습니다!


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.

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

@unam98 unam98 force-pushed the develop branch 2 times, most recently from 9538b84 to 55b4ded Compare December 20, 2023 13:17
@unam98 unam98 force-pushed the develop branch 3 times, most recently from fb1f98f to f8525c2 Compare June 19, 2024 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MOD ☁ 코드 및 내부 파일 수정 REFACTOR 🧹 코드 리팩토링 우남 🐼 우남 담당
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

[REFACTOR] 출발지 모드 선택 / data class에서 'mode' 제거 후 따로 intent로 넘기기
2 participants