manas-chaudhari / android-mvvm

MVVM on Android using RxJava and Data Binding
Apache License 2.0
502 stars 81 forks source link

Restructure Project #40

Open manas-chaudhari opened 8 years ago

manas-chaudhari commented 8 years ago

Because MVVM & RxJava Databinding integration are two streams, which can be used separately, it is possible to split these into two separate libraries.

  1. android-mvvm
    • The generic Adapters & binding adapters
    • Perhaps RxJava's dependency could be removed from this part by replacing rx.Observable<List<ViewModel>> with databinding.ObservableList<>
    • Split library into modules based on support libraries, so that there is no overhead of including an indirect dependency.
  2. databinding-rxjava
    • Conversion between rx.Observable and databinding.ObservableField
    • Additionally, bindingConversions can be provided to convert from Observable<List<>> to ObservableList<>
      • This would be essential for Adapters in MVVM to work with Rx
  3. databinding-rxjava2
    • Same as above but with RxJava2 support
  4. databinding-rxjava-kotlin
    • Extension functions for easy conversions
  5. databinding-rxjava2-kotlin
    • Same as above, but with RxJava2 support
manas-chaudhari commented 7 years ago

There are few issues that will come up if RecyclerViewAdapter argument is changed to databinding.ObservableList.

The type in BindingAdapter would also change to ObservableList.

public void bindAdapter(RecyclerView, ObservableList<ViewModel>, ViewProvider viewProvider)

With ObservableList, data binding invokes the adapter every time the list changes. Currently, the RecyclerViewAdapter also internally observes the provided list and notifies itself. This will add to redundancy. There are few ways to go ahead:

1. Do nothing when BindingAdapter is called repeatedly

One way around this can be to add a condition in the binding adapter which will compare the provided observableList to the old value and create adapter only if they different.

This approach works because Data Binding will provide an instance of ObservableList to the BindingAdapter. However, if it is required to pass another changing value which we want the RecyclerView to observe internally, we'll want to have an ObservableField<> (instead of rx.Observable) as an argument. This cannot be achieved because Data Binding only passes the inner data to the binding adapter.

As a result, using databinding.ObservableXYZ as arguments in BindingAdapter would result in severe limitations. Perhaps ObservableList is the only type that can be supported.

2. Remove Observation from RecyclerViewAdapter

Another way is to remove internal observation from the RecyclerViewAdapter. The BindingAdapter would pass the new list and notify the adapter whenever it gets called.

  public static void bind(RecyclerView rv, List<ViewModel> items, ...) {
      RecyclerViewAdapter = rv.getAdapter(); 
      // Create adapter if not present

      rv.setItems(items);
      rv.notifyDataSetChanged():
  }

This approach nicely decouples observation and adapter logic. Thus, any additional arguments (if required) can be passed to the BindingAdapter without wrapping into Observable and invoke setters of the adapter.

Limitation of this is that it could become difficult (though not impossible) to extend RecyclerViewAdapter functionality. Example: Let's say you want animations synced with the changes in the list. Because data binding does not provide information related to what has changed in the list in the BindingAdapter, animations cannot be implemented. In approach 1, because RecyclerView is adding listener to the ObservableList, it is free to do the animations if required.

This limitation can be overcome by wrapping the change information into new types. For example, Instead of maintaining ObservableList<String>, one can implement ObservableField<ListEvent<String>>. ListEvent class will contain information about what has changed in the list and also a reference to the latest list.

public static void bind(RecyclerView rv, ListEvent<ViewModel> listEvent, ...) {
      RecyclerViewAdapter = rv.getAdapter(); 
      // Create adapter if not present

      if (listEvent.type == ADDED) {
              rv.notifyItemRangeInserted(listEvent.position, listEvent.count)
      }
      ...
}

This needs more thought. But it seems doable.

3. Let the Rx dependency remain in the core

This would mean the core module would get upgraded to use RxJava2 and another module will provide BindingConversion for working with RxJava1 using https://github.com/akarnokd/RxJava2Interop.


Approach 1 has severe limitations. Approach 2 would be a big step and needs more validation. Approach 3 is not actually refactoring anything. It simply ports to RxJava2 retaining the current structure.

manas-chaudhari commented 7 years ago

This article https://medium.com/google-developers/android-data-binding-list-tricks-ef3d5630555e#.x3q5qels4 uses approach 1.

@BindingAdapter({"entries", "layout"})
public static <T> void setEntries(ViewGroup viewGroup,
        ObservableList<T> oldEntries, int oldLayoutId,
        ObservableList<T> newEntries, int newLayoutId) {
    if (oldEntries == newEntries && oldLayoutId == newLayoutId) {
        return; // nothing has changed
    }
    // refresh here

The behavior for entries and layoutId is not symmetric. entries type is ObservableList, hence the binding adapter adds its own listener to it. This allows the adapter to get more information about the changes in the list, which it uses to update the views. When list's internal data changes, the bindingAdapter doesn't do any new work. The listener that it had added previously does all the work. However, when layoutId changes, the BindingAdapter does the work of updating.

On a different note

The article implements list binding for LinearLayout, which is very interesting. It would be a great feature to include in this library. However, I feel there's a lot of code repetition between different adapters. Implementation for LinearLayout OR other ViewGroup, would result in yet other repetition. It would be awesome if a common abstract adapter is written which all existing adapters will make use of. Additionally, the abstract adapter would be an additional tool for library users to easily build their own Adapters for View Composition. This strengthens the argument in #13