fornewid / naver-map-compose

NAVER Map Android SDK for Jetpack Compose 🗺
https://fornewid.github.io/naver-map-compose/
Apache License 2.0
124 stars 7 forks source link

code cleanup #18

Closed jisungbin closed 2 years ago

jisungbin commented 2 years ago
  1. 리드미에서 마크다운 린트에 맞게 줄 간격을 한 줄 추가하였습니다.
  2. NaverMap.ktmapClickListeners 부분에서 remember 사용을 최적화 하였습니다. remember는 한 번만 인스턴스를 할당하고 key가 바뀌지 않는 한 인스턴스를 그대로 재사용하는 목적으로 알고 있습니다. 기존처럼 하게 되면 내부 옵션(onMapClick, onMapLongClick, onMapDoubleTab 등등)들이 바뀌지 않아도 매번 리컴포지션마다 인스턴스에 해당 옵션들이 재설정 됩니다. 이러한 옵션들을 remember의 key로 받고, key에 변경이 있을때만 옵션 재설정을 해주게 하는것이 더 좋은 코드일거 같습니다.
  3. ReusableComposeNode가 아니라 ComposeNode만을 쓰셨던데 Reusable을 사용하지 않으신 이유를 알 수 있을까요? 제가 Reusable이 무슨 뜻인지 이해를 못했던 부분이라 이 부분은 알아가고 싶습니다.
jisungbin commented 2 years ago

모든 UI Composable 은 Layout composable 을 호출하는걸로 보이고, Layout composable 은 ReusableComposeNode 를 사용합니다. 그래서 말씀해주신 "ReusableComposeNode은 일반적으로 쓰지 않는 api로 보입니다." 에서 일반적이 무엇을 의미하시는건지 모르겠습니다.

내부 코드는 지금도 열심히 보고는 있지만 역시 혼자서 다 파악하려기엔 무리가 좀 있네요 ㅠ "recomposition에서 factory가 호출되지 않는것 같네요." 는 알려주셔서 감사합니다!

2번 변경 사항은 원복 해두었습니다.

fornewid commented 2 years ago

@jisungbin 제가 얘기드린 일반적이라는 것은, "보통 Jetpack Compose 앱/라이브러리를 개발할 때 사용하는가"의 의미였습니다.

말씀하신 ReusableComposeNode는 internal에서도 한곳 외에는 사용하지 않기 때문에 일반적이지 않은 경우로 보인다고 얘기드렸어요. :eyes:

jisungbin commented 2 years ago

@jisungbin 제가 얘기드린 일반적이라는 것은, "보통 Jetpack Compose 앱/라이브러리를 개발할 때 사용하는가"의 의미였습니다.

말씀하신 ReusableComposeNode는 internal에서도 한곳 외에는 사용하지 않기 때문에 일반적이지 않은 경우로 보인다고 얘기드렸어요. 👀

아하 감사합니다! 그럼 맞는 말씀이신거 같네요 :)