realm / realm-java

Realm is a mobile database: a replacement for SQLite & ORMs
http://realm.io
Apache License 2.0
11.47k stars 1.75k forks source link

Better Databinding Support #4534

Open cmelchior opened 7 years ago

cmelchior commented 7 years ago

Playing around with databinding in a side project made me think about this a bit more. Sorry for the wall of text :)

Databinding is a fairly complex topic especially on when you want to support field change notifications. Right now it requires quite a lot of boilerplate: https://github.com/Zhuinden/realm-databind-experiment/blob/master/app/src/main/java/com/zhuinden/realmdatabind/realm/objects/Post.java

And property notifications needs to be called from a custom listener as well:

// Notify about writes should be done from listeners
Person p = realm.where(Person.class).findFirst();
p.addChangeListner(new RealmObjectChangeListener() {
    @Override
    public void onChange(Person p, ObjectChangeSet changeSet) {
        if (changeSet.isFieldChanged("name")) {
            p.notifyPropertyChanged(BR.name);
        }
    }
});

There is at least 3 ways we can improve this, 2 that are very easy, and one slightly more difficult, but it would make databinding support rather nice.

1. Support BaseObservable class

public class Person extends BaseObservable implements RealmModel {

    private String name;

    @Bindable
    public String getName() {
        return name;
    }

    @Bindable({"name"})
    public String getDisplayName() {
        return name.toLowerCase();
    }
}

// Usage
MyFragmentBinding binder = MyFragmentBinding.inflate(inflater);
binder.setPerson(realm.where(Person.class).findFirst());

Requirements: 1) We must allow people to extend from BaseObservable, or more generally from any non-realm super class. Right now this would actually also work with our type system rather nicely, since people couldn't query for the abstract classes since they wouldn't be implementing RealmModel. This will probably change with #575 though where we would loosen the generics. The quick solution would be to just make an exception for BaseObservable. 2) People must use the RealmModel interface approach (annoying if you don't use Kotlin).

2. Support new RealmBaseObservable class

More convenient:

public class Person extends RealmBaseObservable {

    private String name;

    @Bindable
    public String getName() {
        return name;
    }

    @Bindable({"name"})
    public String getDisplayName() {
        return name.toLowerCase();
    }
}

Requirements: 1) We must provide a RealmBaseObservable class that is our implementation of BaseObservable which also extends RealmObject. This means people could still use the more fluent methods on RealmObject. 2) This class should probably be in realm-android-adapters. 3) We need to extend the annotation processor to be aware of this class, and we need to prevent people from making queries on it.

3. Automatically create RealmObjectChangelistener

Databinding annotation processors will consume the @Bindable annotations which prevents us from creating one that process only those classes, but we can create one similar to https://github.com/cmelchior/realmfieldnameshelper that will process all Realm Model class and check for the @Bindable annotation that way.

Field observing in databinding would be a 1-liner and really showcase how useful object listeners can be.

// This class could be created by an annotation processor since we ca deduce the field/BR name from the @Bindable method
public class PersonObjectChangeListener implements RealmObjectChangeListener<Person> {
    @Override
    public void onChange(Person obj, ObjectChangeSet objectChangeSet) {
        if (objectChangeSet.isFieldChanged("name")) {
            obj.notifyPropertyChanged(dk.ilios.realmviewmodels.example.BR.name);
        }
    }
}

// Usage options
// Like asObservable() but will automatically attach the listener
// Will mean our core library need to know about Databinding
Person p = person.asViewModel(); 

// This could be implemented as a completely seperate project or more likely in realm-android-adapters
Person p = ViewModelFactory.create(person); 

// Perhaps instead
Person p = RealmAndroid.getViewModel(person);

// Just create public classes and let people hook them up themselves
Person p = getPerson();
p.addChangeListener(new PersonObjectChangeListener());

// Example of usage in a fragment/activity
MyFragmentBinding binder = MyFragmentBinding.inflate(inflater);
binder.setPerson(realm.where(Person.class).findFirst().asViewModel());

Requirements: 1) We need an extra annotation processor somewhere. Either as part of our standard one or a new one in realm-android-adapters. 2) That annotation processor need to know the manifest package name which is rather hard without hacks or a gradle plugin, which only our main project uses. See e.g. https://github.com/androidannotations/androidannotations/blob/558b9088fdc2cd82988445ed86d60340b143a95f/AndroidAnnotations/androidannotations-core/androidannotations/src/main/java/org/androidannotations/internal/helper/AndroidManifestFinder.java

Conclussion

1/2 seems rather straight forward and easy to implement. 3) is doable as well, the biggest issue is around how we actually want to distribute it and how much we want to expose Android specific concepts in our API's.

Zhuinden commented 7 years ago

on top of 1), what I initially wanted to do in order to reduce boilerplate was to do this:

public class RealmBaseObservable extends RealmObject implements Observable {
    // stuff from `BaseObservable` goes here, all fields are transient or marked with `@Ignore`
}

public class Post extends RealmBaseObservable { 
    // ...
}

that way implements RealmModel is not needed, but there are still no inherited fields that would mess with the realm schema.

cmelchior commented 7 years ago

Yup that is actually what 2) describes.

Zhuinden commented 7 years ago

Except personally I was thinking that if the parent classes have no inherited fields, then this class would work even without it being explicitly provided by Realm - the support for extending abstract classes with no fields that extend from RealmObject :)

But I don't know if the annotation processor has that sort of power.

cmelchior commented 7 years ago

The annotation processor can detect that. At that point we are losing the help from the type system though and might as well just allow all abstract classes with fields. We would still not support polymorphism (i.e. multiple types in the same collection), but it the feature would be the same as what Cocoa does right now.

seb-saletes commented 7 years ago

Hi guys,

I have an app with a huge form and I'm using two-way data binding. Considering that I'm pretty new to Android development (so pretty slow to implement basics things), Is it worth spending the time to understand this POC https://medium.com/@Zhuinden/realm-1-2-0-android-data-binding-1dc06822287f and implement it in my app ?

Any bits of advice would be greatly appreciated.

Thanks

Zhuinden commented 7 years ago

Considering that example assumes that you only do two-way databinding only with unmanaged objects, it's a bit tricky. All modifications of RealmObjects must be done in a transaction after all.

seb-saletes commented 7 years ago

Thanks for your quick reply.

I'm thinking of using an intermediate Model between my view and my RealmObject. The realmObject and my intermediate Model would have the same attributes. That way I can still be using two-way data binding with my intermediate Model and when the user validates the form I can transfer all the attribute in a transaction to my RealmObject.

I know this is not the best solution but it's really easy to implement. Do you think this is really bad if I do that ?

Zhuinden commented 7 years ago

Depends. If you use an intermediate object, it may as well be an unmanaged RealmObject.

Zhuinden commented 7 years ago

From Realm.NET:

Additionally, we're releasing a new platform specific DataBinding package that contains helper methods that enable two-way databinding scenarios by automatically creating transactions when setting a property.

Apparently they have some sort of magic going on where they automatically begin and commit a transaction whenever a property is changed in databinding.

I can't help but feel like that would be wasteful, hence why I'm surprised to see it.

nirinchev commented 7 years ago

As the person who was driving the databinding support for .NET, I can briefly explain the motivation for our decisions. Databinding is somewhat magical in Xaml - there's a set of interfaces you need to implement (that Realm objects and collections implement those out of the box) and the databinding engine will subscribe for events, update the view and so on. On the other hand, when the UI changes, it will call the property setters directly, and those are generated by the Realm SDK, so the user can't add their custom logic there. This leaves users with no opportunity to begin/commit transactions in response to databinding events, which would be very poor experience.

The way two-way databinding works in the .NET SDK is, we implement an additional interface that is used by the Xaml engine and provide custom property setters that the databinding engine will invoke instead of the regular ones. In those property setters, we check if the Realm is in transaction and if it is, we just call the original setter. If it's not, we wrap the setter call in a transaction, that is committed immediately.

Our reasoning was that

  1. We still give people chance to "optimize" their transactions. If you expect a lot of properties to be changed and don't care for preserving partial changes/drafts (e.g. create new User screen), you can begin a transaction when the screen is displayed, have the user set all fields and commit the transaction on save button click. In that case, you'll group everything in a single transaction, and if the user clicks cancel, or leaves the app, nothing will be persisted in Realm.

  2. We allow people to not worry about starting transactions when they are on a screen where they don't expect a lot of data changes or they are interested in persisting every change - e.g. settings screen. Regarding performance, it's a bit wasteful to do that, indeed, but the performance cost of starting and committing a transaction is negligible compared to the cost of user interaction and the binding engine redrawing stuff.

In conclusion, we feel that the potential performance hit from automating transactions in databinding scenarios is many times offset by the ease of use of the SDK. Here's a blog post I wrote around the time we launched this feature, that showcases the simplicity of the API when using this feature. Hope this helps :)