Move more into the base class

samtstern · samtstern · commit d2fa47e4ed28 · 2017-08-08T08:45:47.000-07:00
Change-Id: I5153b73dff61ac0052838980dfb8a61d816bebc5
diff --git a/common/src/main/java/com/firebase/ui/common/BaseObservableSnapshotArray.java b/common/src/main/java/com/firebase/ui/common/BaseObservableSnapshotArray.java
@@ -15,7 +15,8 @@
  * @param <L> the listener class.
  * @param <E> the model object class.
  */
-public abstract class BaseObservableSnapshotArray<S, L, E> extends AbstractList<S> {
+public abstract class BaseObservableSnapshotArray<S, L extends BaseChangeEventListener<S,?>, E>
+        extends AbstractList<S> {
 
     private final List<L> mListeners = new CopyOnWriteArrayList<>();
     private BaseSnapshotParser<S, E> mParser;
@@ -149,6 +150,24 @@ public int size() {
         return getSnapshots().size();
     }
 
+    protected void notifyListenersOnChildChanged(ChangeEventType type,
+                                                 S snapshot,
+                                                 int newIndex,
+                                                 int oldIndex) {
+        for (L listener : getListeners()) {
+            listener.onChildChanged(type, snapshot, newIndex, oldIndex);
+        }
+    }
+
+    protected void notifyListenersOnDataChanged() {
+        mHasDataChanged = true;
+
+        for (L listener : getListeners()) {
+            listener.onDataChanged();
+        }
+    }
+
+
     // TODO(samstern): Do we need this?
     protected abstract List<S> getSnapshots();
 
diff --git a/database/src/main/java/com/firebase/ui/database/FirebaseArray.java b/database/src/main/java/com/firebase/ui/database/FirebaseArray.java
@@ -88,23 +88,23 @@ public void onChildAdded(DataSnapshot snapshot, String previousChildKey) {
 
         mSnapshots.add(index, snapshot);
 
-        notifyChangeEventListeners(ChangeEventType.ADDED, snapshot, index);
+        notifyListenersOnChildChanged(ChangeEventType.ADDED, snapshot, index, -1);
     }
 
     @Override
     public void onChildChanged(DataSnapshot snapshot, String previousChildKey) {
         int index = getIndexForKey(snapshot.getKey());
 
         updateData(index, snapshot);
-        notifyChangeEventListeners(ChangeEventType.CHANGED, snapshot, index);
+        notifyListenersOnChildChanged(ChangeEventType.CHANGED, snapshot, index, -1);
     }
 
     @Override
     public void onChildRemoved(DataSnapshot snapshot) {
         int index = getIndexForKey(snapshot.getKey());
 
         removeData(index);
-        notifyChangeEventListeners(ChangeEventType.REMOVED, snapshot, index);
+        notifyListenersOnChildChanged(ChangeEventType.REMOVED, snapshot, index, -1);
     }
 
     @Override
@@ -115,10 +115,8 @@ public void onChildMoved(DataSnapshot snapshot, String previousChildKey) {
         int newIndex = previousChildKey == null ? 0 : (getIndexForKey(previousChildKey) + 1);
         mSnapshots.add(newIndex, snapshot);
 
-        notifyChangeEventListeners(ChangeEventType.MOVED,
-                                   snapshot,
-                                   newIndex,
-                                   oldIndex);
+        notifyListenersOnChildChanged(ChangeEventType.MOVED, snapshot,
+                newIndex, oldIndex);
     }
 
     @Override
diff --git a/database/src/main/java/com/firebase/ui/database/FirebaseIndexArray.java b/database/src/main/java/com/firebase/ui/database/FirebaseIndexArray.java
@@ -178,7 +178,7 @@ protected void onKeyMoved(DataSnapshot data, int index, int oldIndex) {
             DataSnapshot snapshot = removeData(oldIndex);
             mHasPendingMoveOrDelete = true;
             mDataSnapshots.add(index, snapshot);
-            notifyChangeEventListeners(ChangeEventType.MOVED, snapshot, index, oldIndex);
+            notifyListenersOnChildChanged(ChangeEventType.MOVED, snapshot, index, oldIndex);
         }
     }
 
@@ -190,7 +190,7 @@ protected void onKeyRemoved(DataSnapshot data, int index) {
         if (isKeyAtIndex(key, index)) {
             DataSnapshot snapshot = removeData(index);
             mHasPendingMoveOrDelete = true;
-            notifyChangeEventListeners(ChangeEventType.REMOVED, snapshot, index);
+            notifyListenersOnChildChanged(ChangeEventType.REMOVED, snapshot, index, -1);
         }
     }
 
@@ -235,12 +235,12 @@ public void onDataChange(DataSnapshot snapshot) {
                 if (isKeyAtIndex(key, index)) {
                     // We already know about this data, just update it
                     updateData(index, snapshot);
-                    notifyChangeEventListeners(ChangeEventType.CHANGED, snapshot, index);
+                    notifyListenersOnChildChanged(ChangeEventType.CHANGED, snapshot, index, -1);
                     notifyListenersOnDataChanged();
                 } else {
                     // We don't already know about this data, add it
                     mDataSnapshots.add(index, snapshot);
-                    notifyChangeEventListeners(ChangeEventType.ADDED, snapshot, index);
+                    notifyListenersOnChildChanged(ChangeEventType.ADDED, snapshot, index, -1);
 
                     mKeysWithPendingData.remove(key);
                     if (mKeysWithPendingData.isEmpty()) notifyListenersOnDataChanged();
@@ -249,7 +249,7 @@ public void onDataChange(DataSnapshot snapshot) {
                 if (isKeyAtIndex(key, index)) {
                     // This data has disappeared, remove it
                     removeData(index);
-                    notifyChangeEventListeners(ChangeEventType.REMOVED, snapshot, index);
+                    notifyListenersOnChildChanged(ChangeEventType.REMOVED, snapshot, index, -1);
                     notifyListenersOnDataChanged();
                 } else {
                     // Data does not exist
diff --git a/database/src/main/java/com/firebase/ui/database/ObservableSnapshotArray.java b/database/src/main/java/com/firebase/ui/database/ObservableSnapshotArray.java
@@ -69,29 +69,7 @@ protected void onListenerAdded(ChangeEventListener listener) {
         }
     }
 
-    protected final void notifyChangeEventListeners(ChangeEventType type,
-                                                    DataSnapshot snapshot,
-                                                    int index) {
-        notifyChangeEventListeners(type, snapshot, index, -1);
-    }
-
-    protected final void notifyChangeEventListeners(ChangeEventType type,
-                                                    DataSnapshot snapshot,
-                                                    int index,
-                                                    int oldIndex) {
-        for (ChangeEventListener listener : getListeners()) {
-            listener.onChildChanged(type, snapshot, index, oldIndex);
-        }
-    }
-
-    protected final void notifyListenersOnDataChanged() {
-        setHasDataChanged(true);
-        for (ChangeEventListener listener : getListeners()) {
-            listener.onDataChanged();
-        }
-    }
-
-    protected final void notifyListenersOnCancelled(DatabaseError error) {
+    protected void notifyListenersOnCancelled(DatabaseError error) {
         for (ChangeEventListener listener : getListeners()) {
             listener.onError(error);
         }
diff --git a/firestore/src/main/java/com/firebase/ui/firestore/FirestoreArray.java b/firestore/src/main/java/com/firebase/ui/firestore/FirestoreArray.java
@@ -103,15 +103,15 @@ public void onEvent(QuerySnapshot snapshots, FirebaseFirestoreException e) {
             }
         }
 
-        notifyOnDataChanged();
+        notifyListenersOnDataChanged();
     }
 
     private void onDocumentAdded(DocumentChange change) {
         Log.d(TAG, "Added: " + change.getNewIndex());
 
         // Add the document to the set
         mSnapshots.add(change.getNewIndex(), change.getDocument());
-        notifyOnChildChanged(ChangeEventType.ADDED, change.getDocument(),
+        notifyListenersOnChildChanged(ChangeEventType.ADDED, change.getDocument(),
                 change.getNewIndex(), -1);
     }
 
@@ -123,7 +123,7 @@ private void onDocumentRemoved(DocumentChange change) {
 
         // Remove the document from the set
         mSnapshots.remove(change.getOldIndex());
-        notifyOnChildChanged(ChangeEventType.REMOVED, change.getDocument(),
+        notifyListenersOnChildChanged(ChangeEventType.REMOVED, change.getDocument(),
                 -1, change.getOldIndex());
     }
 
@@ -136,14 +136,14 @@ private void onDocumentModified(DocumentChange change) {
             Log.d(TAG, "Modified (inplace): " + change.getOldIndex());
 
             mSnapshots.set(change.getOldIndex(), change.getDocument());
-            notifyOnChildChanged(ChangeEventType.CHANGED, change.getDocument(),
+            notifyListenersOnChildChanged(ChangeEventType.CHANGED, change.getDocument(),
                     change.getNewIndex(), change.getOldIndex());
         } else {
             Log.d(TAG, "Modified (moved): " + change.getOldIndex() + " --> " + change.getNewIndex());
 
             mSnapshots.remove(change.getOldIndex());
             mSnapshots.add(change.getNewIndex(), change.getDocument());
-            notifyOnChildChanged(ChangeEventType.MOVED, change.getDocument(),
+            notifyListenersOnChildChanged(ChangeEventType.MOVED, change.getDocument(),
                     change.getNewIndex(), change.getOldIndex());
         }
     }
@@ -171,24 +171,10 @@ private void stopListening() {
         mSnapshots.clear();
     }
 
-    private void notifyOnChildChanged(ChangeEventType type,
-                                      DocumentSnapshot snapshot,
-                                      int newIndex, int oldIndex) {
-
-        for (ChangeEventListener listener : getListeners()) {
-            listener.onChildChanged(type, snapshot, newIndex, oldIndex);
-        }
-    }
-
+    // TODO: There should be a way to move this into the base class
     private void notifyOnError(FirebaseFirestoreException e) {
         for (ChangeEventListener listener : getListeners()) {
             listener.onError(e);
         }
     }
-
-    private void notifyOnDataChanged() {
-        for (ChangeEventListener listener : getListeners()) {
-            listener.onDataChanged();
-        }
-    }
 }

Original file line number	Diff line number	Diff line change
`@@ -103,15 +103,15 @@ public void onEvent(QuerySnapshot snapshots, FirebaseFirestoreException e) {`
`103`	`103`	`}`
`104`	`104`	`}`
`105`	`105`
`106`		`- notifyOnDataChanged();`
	`106`	`+ notifyListenersOnDataChanged();`
`107`	`107`	`}`
`108`	`108`
`109`	`109`	`private void onDocumentAdded(DocumentChange change) {`
`110`	`110`	`Log.d(TAG, "Added: " + change.getNewIndex());`
`111`	`111`
`112`	`112`	`// Add the document to the set`
`113`	`113`	`mSnapshots.add(change.getNewIndex(), change.getDocument());`
`114`		`- notifyOnChildChanged(ChangeEventType.ADDED, change.getDocument(),`
	`114`	`+ notifyListenersOnChildChanged(ChangeEventType.ADDED, change.getDocument(),`
`115`	`115`	`change.getNewIndex(), -1);`
`116`	`116`	`}`
`117`	`117`
`@@ -123,7 +123,7 @@ private void onDocumentRemoved(DocumentChange change) {`
`123`	`123`
`124`	`124`	`// Remove the document from the set`
`125`	`125`	`mSnapshots.remove(change.getOldIndex());`
`126`		`- notifyOnChildChanged(ChangeEventType.REMOVED, change.getDocument(),`
	`126`	`+ notifyListenersOnChildChanged(ChangeEventType.REMOVED, change.getDocument(),`
`127`	`127`	`-1, change.getOldIndex());`
`128`	`128`	`}`
`129`	`129`
`@@ -136,14 +136,14 @@ private void onDocumentModified(DocumentChange change) {`
`136`	`136`	`Log.d(TAG, "Modified (inplace): " + change.getOldIndex());`
`137`	`137`
`138`	`138`	`mSnapshots.set(change.getOldIndex(), change.getDocument());`
`139`		`- notifyOnChildChanged(ChangeEventType.CHANGED, change.getDocument(),`
	`139`	`+ notifyListenersOnChildChanged(ChangeEventType.CHANGED, change.getDocument(),`
`140`	`140`	`change.getNewIndex(), change.getOldIndex());`
`141`	`141`	`} else {`
`142`	`142`	`Log.d(TAG, "Modified (moved): " + change.getOldIndex() + " --> " + change.getNewIndex());`
`143`	`143`
`144`	`144`	`mSnapshots.remove(change.getOldIndex());`
`145`	`145`	`mSnapshots.add(change.getNewIndex(), change.getDocument());`
`146`		`- notifyOnChildChanged(ChangeEventType.MOVED, change.getDocument(),`
	`146`	`+ notifyListenersOnChildChanged(ChangeEventType.MOVED, change.getDocument(),`
`147`	`147`	`change.getNewIndex(), change.getOldIndex());`
`148`	`148`	`}`
`149`	`149`	`}`
`@@ -171,24 +171,10 @@ private void stopListening() {`
`171`	`171`	`mSnapshots.clear();`
`172`	`172`	`}`
`173`	`173`
`174`		`- private void notifyOnChildChanged(ChangeEventType type,`
`175`		`- DocumentSnapshot snapshot,`
`176`		`- int newIndex, int oldIndex) {`
`177`		`-`
`178`		`- for (ChangeEventListener listener : getListeners()) {`
`179`		`- listener.onChildChanged(type, snapshot, newIndex, oldIndex);`
`180`		`- }`
`181`		`- }`
`182`		`-`
	`174`	`+ // TODO: There should be a way to move this into the base class`
`183`	`175`	`private void notifyOnError(FirebaseFirestoreException e) {`
`184`	`176`	`for (ChangeEventListener listener : getListeners()) {`
`185`	`177`	`listener.onError(e);`
`186`	`178`	`}`
`187`	`179`	`}`
`188`		`-`
`189`		`- private void notifyOnDataChanged() {`
`190`		`- for (ChangeEventListener listener : getListeners()) {`
`191`		`- listener.onDataChanged();`
`192`		`- }`
`193`		`- }`
`194`	`180`	`}`