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

Fine grained notification for RecyclerViewAdapter #83

Merged
merged 6 commits into from Feb 28, 2017

Conversation

beeender
Copy link
Contributor

No description provided.

@@ -87,10 +87,12 @@ public void onChange(BaseRealm results) {
private void addListener(@NonNull OrderedRealmCollection<T> data) {
if (data instanceof RealmResults) {
Copy link
Contributor

Choose a reason for hiding this comment

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

check for instance of RealmCollectionObservable instead? and collapse the two if sections?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ignore this after removing this interface

- Refresh only the needed part in RecyclerViewAdapter acording to the
  given change set.
- Change Timestamp to Counter and show an ordered results in the views.
  To show a better experience for recycler view.
- Add a edit button to randomly delete/add objects in recycler view.
- Remove deprecated methods.
@cmelchior
Copy link
Contributor

Currently crashes with:

02-27 09:44:35.634  2781  2794 E AndroidRuntime: FATAL EXCEPTION: Instr: android.support.test.runner.AndroidJUnitRunner
02-27 09:44:35.634  2781  2794 E AndroidRuntime: Process: io.realm.android.tests.test, PID: 2781
02-27 09:44:35.634  2781  2794 E AndroidRuntime: java.lang.IncompatibleClassChangeError: io.realm.AndroidNotifier
02-27 09:44:35.634  2781  2794 E AndroidRuntime: 	at dalvik.system.DexFile.defineClassNative(Native Method)
02-27 09:44:35.634  2781  2794 E AndroidRuntime: 	at dalvik.system.DexFile.defineClass(DexFile.java:226)
02-27 09:44:35.634  2781  2794 E AndroidRuntime: 	at dalvik.system.DexFile.loadClassBinaryName(DexFile.java:219)
02-27 09:44:35.634  2781  2794 E AndroidRuntime: 	at dalvik.system.DexPathList.findClass(DexPathList.java:338)
02-27 09:44:35.634  2781  2794 E AndroidRuntime: 	at dalvik.system.BaseDexClassLoader.findClass(BaseDexClassLoader.java:54)
02-27 09:44:35.634  2781  2794 E AndroidRuntime: 	at java.lang.ClassLoader.loadClass(ClassLoader.java:511)
02-27 09:44:35.634  2781  2794 E AndroidRuntime: 	at java.lang.ClassLoader.loadClass(ClassLoader.java:469)
02-27 09:44:35.634  2781  2794 E AndroidRuntime: 	at java.lang.Class.classForName(Native Method)
02-27 09:44:35.634  2781  2794 E AndroidRuntime: 	at java.lang.Class.forName(Class.java:324)
02

When running gw clean :tests:connectedCheck. Not entirely sure why.

@zaki50
Copy link
Contributor

zaki50 commented Feb 27, 2017

@cmelchior I could reproduce this with 3.0.0-SNAPSHOT from jfrog but worked fine with my local build of 3.0.0-SNAPSHOT.

Something wrong in SNAPSHOT build?

@cmelchior
Copy link
Contributor

Perhaps. I had to upload the SNAPSHOT from my local machine because CI didn't do it. Can you try running ojoUpload ?


public RealmBaseAdapter(@Nullable OrderedRealmCollection<T> data) {
if (data != null && !data.isManaged())
throw new IllegalStateException("Only use this adapter with managed list, " +
"for un-managed lists you can just use the BaseAdapter");
this.adapterData = data;
this.listener = new RealmChangeListener<BaseRealm>() {
this.listener = new RealmChangeListener<Object>() {
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be RealmChangeListener<OrderedRealmCollection<T>> since addListener can only accept RealmResults or RealmList

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree. I think it's OK to use raw type (RealmChangeListener).
RealmRecyclerViewAdapter also is using raw type.

@@ -85,12 +79,7 @@ public RealmRecyclerViewAdapter(@Nullable OrderedRealmCollection<T> data, boolea
// Right now don't use generics, since we need maintain two different
Copy link
Contributor

Choose a reason for hiding this comment

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

we need to

@@ -85,12 +79,7 @@ public RealmRecyclerViewAdapter(@Nullable OrderedRealmCollection<T> data, boolea
// Right now don't use generics, since we need maintain two different
// types of listeners until RealmList is properly supported.
// See https://github.com/realm/realm-java/issues/989
Copy link
Contributor

Choose a reason for hiding this comment

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

is the above assumption still valid? realm/realm-java#989 is closed now?

@@ -22,11 +25,19 @@ buildscript {
}
}

// Don't cache SNAPSHOT (changing) dependencies.
Copy link
Contributor

Choose a reason for hiding this comment

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

remember to remove when releasing

public class Counter extends RealmObject {
public static final String FIELD_COUNT = "count";

private static AtomicInteger integerCounter = new AtomicInteger(0);
Copy link
Contributor

Choose a reason for hiding this comment

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

why static? can't we use a member counter with @Ignore ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this is in order to have a global counter while the app is running so the list will have ever increasing numbers.

}

public void setTimeStamp(String timeStamp) {
this.timeStamp = timeStamp;
public void setAndIncrease() {
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 increment is probably a more reflective name, as we're not setting any value (method doesn't take args)

realm.executeTransactionAsync(new Realm.Transaction() {
@Override
public void execute(Realm realm) {
Random rand = new Random();
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 Random is a bit expensive to init. better create it as a member I guess.

RealmResults<Counter> results = realm.where(Counter.class).findAllSorted(Counter.FIELD_COUNT);

// Duplicate some existing entries.
int countToDup = results.size() > 3 ? 3 : results.size();
Copy link
Contributor

Choose a reason for hiding this comment

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

countToDup max is 3 (just double checking)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, refactored to named variable for clarity

CHANGELOG.md Outdated

* Removed `RealmBaseAdapter(@Nonnull Context context, @Nullable OrderedRealmCollection<T> data)`.
* Removed `RealmRecyclerViewAdapter(@NonNull Context context, @Nullable OrderedRealmCollection<T> data, boolean autoUpdate)`.

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 removing RealmBaseAdapter#inflater, RealmBaseAdapter#context, RealmRecyclerViewAdapter#inflater and RealmRecyclerViewAdapter#context are also breaking changes.

@@ -1,3 +1,14 @@
## 2.0.0 (YYYY-MM-DD)
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 we should state supporting Realm-Java version at the beginning of each version entry in Changelog

now 3.x?

Copy link
Member

Choose a reason for hiding this comment

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

Sounds like a good idea.

realmList.realm.handlerController.addChangeListenerAsWeakReference(listener);
RealmList<T> list = (RealmList<T>) data;
//noinspection unchecked
list.addChangeListener((RealmChangeListener)listener);
Copy link
Contributor

Choose a reason for hiding this comment

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

need a space after cast


public RealmBaseAdapter(@Nullable OrderedRealmCollection<T> data) {
if (data != null && !data.isManaged())
throw new IllegalStateException("Only use this adapter with managed list, " +
"for un-managed lists you can just use the BaseAdapter");
this.adapterData = data;
this.listener = new RealmChangeListener<BaseRealm>() {
this.listener = new RealmChangeListener<Object>() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Agree. I think it's OK to use raw type (RealmChangeListener).
RealmRecyclerViewAdapter also is using raw type.

// Duplicate some existing entries.
int countToDup = results.size() > 3 ? 3 : results.size();
for (int i = 0; i < countToDup; i++) {
int id = results.get(rand.nextInt((results.size()))).getCount();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why id?

This value is shared among multiple items.
It does not seem id.

@cmelchior
Copy link
Contributor

@nhachicha @zaki50 PR updated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants