WindSekirun / NaraeImagePicker

MultiImagePicker for Android Application, written in Kotlin
Apache License 2.0
40 stars 14 forks source link

Remove kotlin-android-extensions #21

Closed WindSekirun closed 4 years ago

WindSekirun commented 4 years ago

Remove all 'kotlin android extensions' code to avoid memory issues that can occur while caching views.

deepakkumardk commented 4 years ago

Hi @WindSekirun So after removing the kotlin-android-extension are we migrating this to DataBinding? which is good. But I don't understand what can be the memory issues for caching view as kotlin-android-extension remove all the cache view onDestroy() method. Is there any article/source from which I can go through to dig a little deeper on this topic that it can cause some serious issue.

WindSekirun commented 4 years ago

Hi, @deepakkumardk

Starting from this commit message, Kotlin-Android-Extensions is not officially recommended (which is not the same as recommendation against) cause these reasons.

  1. Cache view into HashMap
  2. Doesn't expose nullabilty when views are only present in some configuration.
  3. Risk of making RecyclerView not cache ViewHolder.
  4. If some views have same id, Type inference does not work properly.

Although this problem may not be significant, the risk exists and is intended to be removed.

I consider DataBinding as another alternative, but I try to avoid adding dependencies that may not be used by other applications. (The issue in #22 has the same problem.) findViewByIds with lateinit commands also fine to me. (The code will grow a lot, but it doesn't cause any problems)

You can find source of topic in here and here.

Any other comments are welcome at any time. I don't have enough time to implement right away.

deepakkumardk commented 4 years ago

@WindSekirun Thanks for the info, I will work on that.

WindSekirun commented 4 years ago

24 merged.

deepakkumardk commented 4 years ago

@WindSekirun now we can close this.