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

Implement fine gained notification #4191

Merged
merged 9 commits into from
Feb 23, 2017
Merged

Conversation

beeender
Copy link
Contributor

@beeender beeender commented Feb 16, 2017

  • Add RealmObservable and RealmCollectionObservable interfaces.
  • Enable detailed change information for RealmResults through
    OrderedCollectionChange interface.
  • Fix a bug in the ObserverPairList which could cause the removed
    listener gets called if it was removed during previous listener
    iteration.

Fix #989

  • RealmObject should implement RealmObservable.

@@ -141,7 +141,7 @@ public boolean isInTransaction() {
* @throws IllegalStateException if you try to remove a listener from a non-Looper Thread.
* @see io.realm.RealmChangeListener
*/
public <T extends BaseRealm> void removeChangeListener(RealmChangeListener<T> listener) {
protected <T extends BaseRealm> void removeListener(RealmChangeListener<T> listener) {
Copy link
Contributor

Choose a reason for hiding this comment

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

whitespace

*/
public class OrderedCollectionChange {
public long[] getDeletions() { return null; }
public long[] getInsertertions() { return null; }
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm pretty sure this should be getInsertions() and I eerily remember stating that in the RFC too 👅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought i changed this ...

package io.realm;

public interface RealmObservable<T> {
void addChangeListener(RealmChangeListener<T> listener);
Copy link
Contributor

Choose a reason for hiding this comment

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

should this also have javadoc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, just make it work, will add javadoc later.

@beeender beeender changed the title WIP fine gained notification Implement fine gained notification Feb 17, 2017
*
* @return the indices array.
*/
long[] getChanges();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I feel getModifications() will be a better name, since this class named OrderedCollectionChange. Your thoughts? @realm/java

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds fine with me

Copy link
Contributor

Choose a reason for hiding this comment

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

But considering that RecyclerView has final void notifyItemRangeChanged(int positionStart, int itemCount) method, I think the OrderedCollectionChangeSet + getChanges() is what makes most sense

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree 👍

@beeender
Copy link
Contributor Author

It still needs more documentation and i met some trouble to make the RealmObject implement RealmObservable. Other than those, it is ready for review.


package io.realm;

public interface RealmObservable<T> {
Copy link
Contributor

Choose a reason for hiding this comment

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

An idea for the javadoc.

A class implementing this interface is capable of reporting when the data stored by the class has changed. When that happens all registered {@link RealmChangeListener}'s will be triggered.
<p>
This class will only report that <i>something<i> changed, not what changed. 

@see RealmCollectionObservable for information about more fine-grained collection notifications.


package io.realm;

public interface RealmCollectionObservable<T, S extends OrderedRealmCollectionChangeListener>
Copy link
Contributor

Choose a reason for hiding this comment

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

Idea for Javadoc:

A collection class implementing this interface is capable of reporting fine-grained notifications about how the collection. It will report insertions, deletions and changes, but not _how_ an individual element changed. When a change is detected all registered listeners will be triggered. 
<p>
This is commonly useful when updating UI elements, e.g `RecyclerView.Adapter` can provide nicer animations and work more effectively if it knows exactly which elements changed.

@see RealmObservable for information about more coarse-grained notifications.
@see <a href="https://realm.io/docs/java/latest/#adapters">Android Adapters<a>

*
* @return the {@link Range} array.
*/
Range[] getChangeRanges();
Copy link
Contributor

Choose a reason for hiding this comment

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

getModificationRanges? pr above

Copy link
Contributor

@Zhuinden Zhuinden Feb 17, 2017

Choose a reason for hiding this comment

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

Let's not forget that RecyclerView.Adapter calls it "changes" too

final void notifyDataSetChanged()

Notify any registered observers that the data set has changed.


final void notifyItemChanged(int position, Object payload)

Notify any registered observers that the item at position has changed with an optional payload object.


final void notifyItemChanged(int position)

Notify any registered observers that the item at position has changed.


final void notifyItemInserted(int position)

Notify any registered observers that the item reflected at position has been newly inserted.


final void notifyItemMoved(int fromPosition, int toPosition)

Notify any registered observers that the item reflected at fromPosition has been moved to toPosition.


final void notifyItemRangeChanged(int positionStart, int itemCount, Object payload)

Notify any registered observers that the itemCount items starting at position positionStart have changed.


final void notifyItemRangeChanged(int positionStart, int itemCount)

Notify any registered observers that the itemCount items starting at position positionStart have changed.


final void notifyItemRangeInserted(int positionStart, int itemCount)

Notify any registered observers that the currently reflected itemCount items starting at positionStart have been newly inserted.


final void notifyItemRangeRemoved(int positionStart, int itemCount)

Notify any registered observers that the itemCount items previously located at positionStart have been removed from the data set.


final void notifyItemRemoved(int position)

Notify any registered observers that the item previously located at position has been removed from the data set.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I guess that was where I got the original wording from. In this case though our class is also called OrderedCollectionChange

I don't have a strong preference either way. Calling the class OrderedCollectionModifications doesn't seem much better either.

Copy link
Contributor

Choose a reason for hiding this comment

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

No, it really doesn't. It feels unnecessarily long 😕

* The change information is available in two formats: a simple array of row indices in the collection for each type of
* change, and an array of {@link Range}s.
*/
public interface OrderedCollectionChange {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be Change or Changes since this interface wraps multiple changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or use the same term as the Object store ChangeSet?

Copy link
Contributor

Choose a reason for hiding this comment

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

ChangeSet sounds like a good idea. That way getChanges/getChangeRanges() also makes a lot of sense IMO

* This will be called when the async query finished at the first time or the collection gets changed.
*
* @param collection the collection this listener is registered to.
* @param changes {@code null} if the it is first time async query returns or information about which rows in the
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@realm/java the changes will be null if it is the first time async query returns. It is the same behaviour like cocoa. Anyone think it should return an empty OrderedCollectionChange instead? in that case, we need a new method OrderedCollectionChange.isEmpty() which feels redundant.

Copy link
Contributor

Choose a reason for hiding this comment

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

Even through there is a difference between zero and no elements, I think null is fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

I have mixed feelings about it. Normally forcing null-checks onto users leads to a lot of boilerplate, but in this case it does seem like the easiest way to express "no change happened", so I guess returning null is fine.

/**
* The indices of objects in the previous version of the collection which have been removed from this one.
*
* @return the indices array.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@realm/java This will return null if no deletion in the changeset, feels right?

Copy link
Contributor

Choose a reason for hiding this comment

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

null might be better than an artificial NOT_FOUND value.

Copy link
Contributor

Choose a reason for hiding this comment

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

null is a very bad return value since the canonical use case is something like:

long[] deletions = changes.getDeletions();
for (int i = 0; i < deletions.length; i++) {
  adapter.notifyItemDleted(deletions[i]);
}

IMO returning a 0-sized array would be strongly preferred.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tend to agree with 0 size array will be better. But will it feel inconsistency with #4191 (comment) ?

Copy link
Contributor

Choose a reason for hiding this comment

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

A little perhaps, but then again, they will be used in two different contexts. So probably acceptable.

Copy link
Contributor

Choose a reason for hiding this comment

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

If not, having an isEmpty() doesn't seem that bad to me.

@beeender beeender changed the base branch from mc/collection-snapshot to master February 20, 2017 13:14
- Add RealmObservable and RealmCollectionObservable interfaces.
- Enable detailed change information for RealmResults through
  OrderedCollectionChange interface.
- Fix a bug in the ObserverPairList which could cause the removed
  listener gets called if it was removed during previous listener
  iteration.

Fix #989
Copy link
Contributor

@cmelchior cmelchior left a comment

Choose a reason for hiding this comment

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

Only review first 3/4 parts of this. Will review the rest later. Only minor comments so far though 👍

modifyObjects(realm, 12, 3, 4, 9);
realm.commitTransaction();
// After transaction, '*' means the object has been modified. 12 has been modified as well, but it is created
// and modified in the same transaction, should not be counted in the changes range.
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the same apply if an object is modified-then-deleted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes. Added two tests to ensure that.

});

final CountDownLatch bgDeletionLatch = new CountDownLatch(1);
new Thread(new Runnable() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason this is running in a seperate thread?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

    // beginTransaction() will make the async query return immediately. So we have to create an object in another
    // thread. Also, the latch has to be counted down after transaction committed so the async query results can
    // contain the modification in the background transaction.

Added above comments in test.

try {
std::rethrow_exception(err);
} catch(const std::exception& e) {
realm::jni_util::Log::e("Caught exception in collection change callback %1", e.what());
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume same reason for not throwing in SyncClients error handler?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

/**
* The indices of objects in the previous version of the collection which have been removed from this one.
*
* @return the indices array. A zero-sized array will be returned if no deletion in the change set.
Copy link
Contributor

Choose a reason for hiding this comment

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

if no objects where deleted.

/**
* The indices in the new version of the collection which were newly inserted.
*
* @return the indices array. A zero-sized array will be returned if no insertion in the change set.
Copy link
Contributor

Choose a reason for hiding this comment

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

if no objects were inserted.

* For {@link RealmResults}, this means that one or more of the properties of the object at that index were
* modified (or an object linked to by that object was modified).
*
* @return the indices array. A zero-sized array will be returned if no modification in the change set.
Copy link
Contributor

Choose a reason for hiding this comment

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

if no objects were modified.

/**
*
*/
class Range {
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume we will add Move if needed later? It will be a breaking change in the API at that point, but a very minor one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. But i think the commit with suppressing listeners will be more important for the real use cases than move.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why move will be a breaking change? it only add something to the API without changing others.

Copy link
Contributor

Choose a reason for hiding this comment

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

Because you are adding a method to an interface. If people implemented that interface they will have to add a new method as well. It is minor I know, but in a strictly SemVer sense it is breaking.

*/
public interface OrderedRealmCollectionChangeListener<T> {
/**
* This will be called when the async query finished at the first time or the collection gets changed.
Copy link
Contributor

Choose a reason for hiding this comment

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

This will be called when the async query is finished the first time or the collection of objects has changed.

@@ -61,7 +49,8 @@
* @see RealmQuery#findAll()
* @see Realm#executeTransaction(Realm.Transaction)
*/
public class RealmResults<E extends RealmModel> extends OrderedRealmCollectionImpl<E> {
public class RealmResults<E extends RealmModel> extends OrderedRealmCollectionImpl<E>
implements RealmCollectionObservable<RealmResults<E>, OrderedRealmCollectionChangeListener<RealmResults<E>>> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this just be implements OrderedRealmCollectionChangeListener<RealmResults<E>>> ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It implements RealmCollectionObservable<T, S extends OrderedRealmCollectionChangeListener>. Or I have some misunderstanding?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ups, yeah... those generics are messing with my head 👍

*/
@Deprecated
Copy link
Contributor

Choose a reason for hiding this comment

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

Add this to the changelog

}

if (ranges_vector.size() > io_realm_internal_CollectionChangeSet_MAX_ARRAY_LENGTH) {
ThrowException(env, IllegalState,
Copy link
Contributor

Choose a reason for hiding this comment

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

If ranges_vector.size() and io_realm_internal_CollectionChangeSet_MAX_ARRAY_LENGTH are included in the error message, user might have a better chance to find the bug.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are absolutely right, sir! Changed that.

for (auto index : index_set.as_indexes()) {
indices_vector.push_back(index);
}
if (indices_vector.size() > io_realm_internal_CollectionChangeSet_MAX_ARRAY_LENGTH) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Like above

}

REALM_UNREACHABLE();
return nullptr;
Copy link
Contributor

Choose a reason for hiding this comment

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

Really needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope. will remove.

return index_set_to_jlong_array(env, change_set.insertions);
case io_realm_internal_CollectionChangeSet_TYPE_MODIFICATION:
return index_set_to_jlong_array(env, change_set.modifications_new);
default:
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not move REALM_UNREACHABLE(); into the default block?

case io_realm_internal_CollectionChangeSet_TYPE_MODIFICATION:
return index_set_to_indices_array(env, change_set.modifications_new);
default:
break;
Copy link
Contributor

Choose a reason for hiding this comment

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

Like above

Range[] getInsertionRanges();

/**
* The ranges of objects in the new version of the collection which were modified.
Copy link
Contributor

Choose a reason for hiding this comment

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

Like above

* collection is changed. It will report insertions, deletions and changes, but not _how_ an individual element changed.
* When a change is detected all registered listeners will be triggered.
* <p>
* This is commonly useful when updating UI elements, e.g {@code RecyclerView.Adapter} can provide nicer animations and
Copy link
Contributor

Choose a reason for hiding this comment

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

"commonly" -> "often"
"e.g" -> "e.g."

package io.realm;

/**
* A class implementing this interface is capable of reporting when the data stored by the class has changed. When that
Copy link
Contributor

Choose a reason for hiding this comment

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

"has" -> "have" (I think "data" is plural in this context: https://en.wikipedia.org/wiki/Data_(word))

/**
* Implementation of {@link OrderedCollectionChangeSet}. This class holds a pointer to the Object Store's
* CollectionChangeSet and read from it only when needed. Creating an Java object from JNI when the collection
* notification arrives is avoided since we also support the collection listeners without a change set parameter,
Copy link
Contributor

Choose a reason for hiding this comment

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

"arrives is" -> "arrives, is"

@@ -49,8 +49,8 @@ public ObserverPair(T observer, S listener) {
this.observerRef = new WeakReference<T>(observer);
}

// The two pairs will be treated as the same only when the observers are the same and the listeners are the same
// as well.
// The two pairs will be treated as the same only when the observers are the same and the listeners equal to
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove "to each other"
"equal" -> "are equal"

Copy link
Contributor

@cmelchior cmelchior left a comment

Choose a reason for hiding this comment

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

Apart from a few minor comments

* This will be called when the async query is finished the first time or the collection of objects has changed.
*
* @param collection the collection this listener is registered to.
* @param changes {@code null} if the it is first time async query returns or information about which rows in the
Copy link
Contributor

Choose a reason for hiding this comment

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

I would probably reverse this to make it easier to read:

@param changeSet Object with information about which rows in the collection were added, removed or modified. {@code null} is returned the first time an async query is completed.

* @param changes {@code null} if the it is first time async query returns or information about which rows in the
* collection were added, removed or modified.
*/
void onChange(T collection, OrderedCollectionChangeSet changes);
Copy link
Contributor

Choose a reason for hiding this comment

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

changes -> changeSet, since you did that renaming in the adapter example?


/**
* A collection class implementing this interface is capable of reporting fine-grained notifications about how the
* collection is changed. It will report insertions, deletions and changes, but not _how_ an individual element changed.
Copy link
Contributor

Choose a reason for hiding this comment

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

_how -> <i>how</i>

public interface RealmCollectionObservable<T, S extends OrderedRealmCollectionChangeListener>
extends RealmObservable<T> {
/**
* Adds a change listener to this RealmResults.
Copy link
Contributor

Choose a reason for hiding this comment

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

to this RealmCollection.

*/
public interface RealmObservable<T> {
/**
* Adds a change listener to this {@link RealmResults}, {@link Realm}, {@link DynamicRealm} or {@link RealmObject}.
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing RealmList

// Convert long array returned by the nativeGetXxxRanges() to Range array.
private Range[] longArrayToRangeArray(long[] longArray) {
if (longArray == null) {
// Returns a size 0 array so we know the JNI gets called.
Copy link
Contributor

Choose a reason for hiding this comment

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

know JNI gets called

Copy link
Contributor

@zaki50 zaki50 left a comment

Choose a reason for hiding this comment

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

👍 with some minor comments

CHANGELOG.md Outdated
@@ -3,7 +3,11 @@
### Breaking changes

* `RealmResults.distinct()` returns a new `RealmResults` object instead of filtering on the original object (#2947).
* `RealmResults` is auto-updated continuously. Any transaction on the current thread which may have an impact on the order or elements of the `RealmResults` will change the `RealmResults` immediately instead of change it in the next event loop. The standard `RealmResults.iterator()` will continue to work as normal, which means that you can still delete or modify elements without impacting the iterator. The same is not true for simple for-loops. In some cases a simple for-loop will not work (https://realm.io/docs/java/2.3.1/api/io/realm/OrderedRealmCollection.html#loops), and you must use the new createSnapshot() method.
* `RealmResults` is auto-updated continuously. Any transaction on the current thread which may have an impact on the order or elements of the `RealmResults` will change the `RealmResults` immediately instead of change it in the next event loop. The standard `RealmResults.iterator()` will continue to work as normal, which means that you can still delete or modify elements without impacting the iterator. The same is not true for simple for-loops. In some cases a simple for-loop will not work (https://realm.io/docs/java/latest/api/io/realm/OrderedRealmCollection.html#loops), and you must use the new createSnapshot() method.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that we should revert this since linking to specific version is more suitable for this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i agree. change to 3.0.0

}

// The args should be [startIndex1, length1, startIndex2, length2, ...]
private void checkRanges(OrderedCollectionChangeSet.Range[] ranges, long... indexAndLen) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't indexAndLen be int...?

}

// Creates AllTypes objects with columnLong set to the value elements in indices array.
private void createObjects(Realm realm, long... indices) {
Copy link
Contributor

Choose a reason for hiding this comment

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

indices should be values?

Copy link
Contributor Author

@beeender beeender Feb 22, 2017

Choose a reason for hiding this comment

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

}

// Modifies AllTypes objects which's columnLong is in the indices array.
private void modifyObjects(Realm realm, long... indices) {
Copy link
Contributor

Choose a reason for hiding this comment

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

indices should be values?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

}

// Deletes AllTypes objects which's columnLong is in the indices array.
private void deleteObjects(Realm realm, long... indices) {
Copy link
Contributor

Choose a reason for hiding this comment

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

indices should be values?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is actually indices. RealmList related tests will share the same test, and to RealmList it is indices.

});

final CountDownLatch bgDeletionLatch = new CountDownLatch(1);
// beginTransaction() will make the async query return immediately. So we have to create an object in another
Copy link
Contributor

Choose a reason for hiding this comment

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

create -> delete?

@@ -979,7 +979,7 @@ public void run() {
@UiThreadTest
public void addChangeListener_null() {
try {
collection.addChangeListener(null);
collection.addChangeListener((RealmChangeListener)null);
Copy link
Contributor

Choose a reason for hiding this comment

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

should have a space before null.

@@ -1024,7 +1024,7 @@ public void run() {
@UiThreadTest
public void removeChangeListener_null() {
try {
collection.removeChangeListener(null);
collection.removeChangeListener((RealmChangeListener)null);
Copy link
Contributor

Choose a reason for hiding this comment

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

should have a space before null.

@zaki50
Copy link
Contributor

zaki50 commented Feb 22, 2017

And I think we should add a sample project for fine grained notification and RecyclerView (in another PR).

@beeender
Copy link
Contributor Author

@zaki50 yes, the example is here realm/realm-android-adapters#83

@beeender beeender merged commit cc82903 into master Feb 23, 2017
@beeender beeender removed the S:Review label Feb 23, 2017
@emanuelez emanuelez deleted the mc/fine-grained-notification branch February 27, 2017 13:59
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 16, 2024
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants